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

[Emanuel Peter]

On Wed, 23 Jul 2025 01:52:54 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:
> 
>   Update intrinsicnode.cpp

@jatin-bhateja @vnkozlov @TobiHartmann 
The TemplateFramework fuzzer found a bug in this bugfix:
https://github.com/openjdk/jdk/pull/28062

It is quite subtle, and would have been hard to spot in the review. But we could have done a better job with tests.

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

> 274:       if (maskcon != -1L) {
> 275:         int bitcount = population_count(static_cast<julong>(bt == T_INT ? maskcon & 0xFFFFFFFFL : maskcon));
> 276:         hi = (1UL << bitcount) - 1;

Replace `1UL` -> `1ULL` for Windows.

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

> 377:         // We can further constrain the upper bound of bit compression if the number of bits
> 378:         // which can be set(one) is less than the maximum number of bits of integral type.
> 379:         hi = MIN2((jlong)((1UL << result_bit_width) - 1L), hi);

Replace `1UL` -> `1ULL` for Windows.

test/hotspot/jtreg/compiler/c2/gvn/TestBitCompressValueTransform.java line 541:

> 539:     public long test18(long src, long mask) {
> 540:         src = Math.max(BOUND_LO_L, Math.min(src, BOUND_HI_L));
> 541:         long res = Long.compress(src, mask);

Here and in all other similar tests:
It seems we are only creating "random input ranges" for `src`, and not for `mask`. Bummer :frowning_face:

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

PR Review: https://git.openjdk.org/jdk/pull/23947#pullrequestreview-3402676772
PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2480227530
PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2480228236
PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2480230411