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

[Jatin Bhateja]

On Mon, 5 May 2025 06:49:29 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Hi @eme64 , I have addressed and responded to your comments, please verify.
>
>> @jatin-bhateja Thanks for the updates! I think I now understand everything except this, so we are making good progress 😊
>> 
>> ```
>>       // For upper bound estimation of result value range with a constant input we
>>       // pessimistically pick max_int value to prevent incorrect constant folding
>>       // in case input equals above estimated lower bound.
>>       hi = src_type->hi_as_long() == lo ? hi : src_type->hi_as_long();
>>       hi = result_bit_width < mask_bit_width ? (1L << result_bit_width) - 1 : hi;
>> ```
>> 
>> Can you please explain it with an example, and walk me through the steps to the incorrect constant folding?
> 
> Let's assume the following
>  - The input was a constant value Integer.MIN_VALUE, hence ideal type TypeInt will have both _lo and _hi set to MIN_VALUE,
>  - Currently, _lo value of result value range flip b/w 0 or MIN_VALUE,  lets take that to be MIN_VALUE in our case.
> 
> Earlier _hi value of the result value range was set to _hi value of the source value range.
> 
> `      hi = mask_max_bw < max_bw ? (1L << mask_max_bw) - 1 : src_type->hi_as_long();
> `
> 
> If the result bit width was less than the maximum bit width of the integral type, in that case both _hi and _lo values were being set to MIN_VALUE resulting into a constant value.

> @jatin-bhateja Well, `KnownBits` will only make testing more difficult, it does not remove challenges, rather increases the challenges. At that point we do not only have to test constants, and ranges, but also all sorts of bit patterns.

@eme64 , glad you are picking that up. I don't want to comment on KnownBits on this PR I will add my review suggestions on #17508, Need a lil time to refresh memory,  but we are excited to contribute to it. 

I think for this bug fix, its better to be safe for now, let me update the revision and test.

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

PR Comment: https://git.openjdk.org/jdk/pull/23947#issuecomment-2850658201