RFR: 8350896: Integer/Long.compress gets wrong type from CompressBitsNode::Value [v9]

[Emanuel Peter]

On Mon, 2 Jun 2025 17:58:09 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Hi All,
>> 
>> This bugfix patch fixes incorrect value computation for Integer/Long. compress APIs.
>> 
>> Problems occur with a constant input and variable mask where the input's value is equal to the lower bound of the mask value., In this case, an erroneous value range estimation results in a constant value. Existing value routine first attempts to constant fold the compression operation if both input and compression mask are constant values; otherwise, it attempts to constrain the value range of result based on the upper and lower bounds of mask type.
>> 
>> New IR test covers the issue reported in the bug report along with a case for value range based logic pruning.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix aarch64 failure

A few more comments about the first part. Will now dig into the case where `!mask_type->is_con()` next...

src/hotspot/share/opto/intrinsicnode.cpp line 241:

> 239:   jlong lo = bt == T_INT ? min_jint : min_jlong;
> 240: 
> 241:   if(mask_type->is_con() && mask_type->get_con_as_long(bt) != -1L) {

Now you removed the condition `mask_type->get_con_as_long(bt) != -1L`. Do you know why it was there in the first place?

It seems to me that if `mask_type->get_con_as_long(bt) == -1L`, then we can just return the type of `src`, right?

src/hotspot/share/opto/intrinsicnode.cpp line 292:

> 290:     //     To compute minimum result value we assume all but last read source bit as zero,
> 291:     //     this is because sign bit of result will always be set to 1 while other bit
> 292:     //     corresponding to set mask bit should be zero.

I don't understand, are you talking about `lo` if `mask < 0`? Don't we just keep `lo = type_min`, which is always ok?

-------------

PR Review: https://git.openjdk.org/jdk/pull/23947#pullrequestreview-2892367256
PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2123819264
PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2123813921