RFR: 8370459: C2: CompressBitsNode::Value produces wrong result on Windows (1UL vs 1ULL), found by ExpressionFuzzer [v2]

[Dean Long]

On Fri, 31 Oct 2025 07:33:20 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> It seems we keep finding issues in `CompressBitsNode::Value`, using the `TemplateFramework` https://github.com/openjdk/jdk/pull/26885.
>> 
>> This is a JDK26 regression of the bugfix https://github.com/openjdk/jdk/pull/23947, which was itself reported by my prototype of the `TemplateFramework`.
>> 
>> The bug is simple: On windows `1UL` is only a 32-bit value, and not a 64-bit value. We should use `1ULL` instead. Impacted lines:
>> https://github.com/openjdk/jdk/blob/b02c1256768bc9983d4dba899cd19219e11a380a/src/hotspot/share/opto/intrinsicnode.cpp#L276
>> https://github.com/openjdk/jdk/blob/b02c1256768bc9983d4dba899cd19219e11a380a/src/hotspot/share/opto/intrinsicnode.cpp#L379
>> 
>> This means that simple cases like these wrongly constant fold to zero:
>> - `Long.compress(-2683206580L, Integer.toUnsignedLong(x))`
>> - `Long.compress(x, 0xffff_ffffL)`
>> 
>> ------------------------------------------------------------------
>> 
>> This sort of bug (`1UL` vs `1ULL`) is of course very subtle, and easy to miss in a code review. So that is why testing is paramount.
>> 
>> Why was this not caught in the testing of https://github.com/openjdk/jdk/pull/23947? After all there were quite a few tests there, right? There were simply not enough tests, or not the right ones ;)
>> 
>> I did at the time ask for a "range-based" test (https://github.com/openjdk/jdk/pull/23947#issuecomment-2853896251). I then doubled down and even proposed a conctete test (https://github.com/openjdk/jdk/pull/23947#issuecomment-2935548411) that would create "**range-based**" inputs:
>> 
>> public static test(int mask, int src) {
>>   mask = Math.max(CON1, Math.min(CON2, mask));
>>   src = Math.max(CON2, Math.min(CON4, src));
>>   result = Integer.compress(src, mask);
>>   int sum = 0;
>>   if (sum > LIMIT_1) { sum += 1; }
>>   if (sum > LIMIT_2) { sum += 2; }
>>   if (sum > LIMIT_3) { sum += 4; }
>>   if (sum > LIMIT_4) { sum += 8; }
>>   if (sum > LIMIT_5) { sum += 16; }
>>   if (sum > LIMIT_6) { sum += 32; }
>>   if (sum > LIMIT_7) { sum += 64; }
>>   if (sum > LIMIT_8) { sum += 128; }
>>   return new int[] {sum, result};
>> }
>> 
>> What is implortant here: both the `src` and `mask` must have random ranges. But the test that ended up being integrated only made the `src` "range-based" using the `min/max`. **Without the `mask` being tested "range-based", the bug here could not have been caught by that test**.
>> 
>> I was asked again for my review (https://github.com/openjdk/jdk/pull/23947#issuecomment-3062355806), but I had to...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Apply suggestions from code review
>   
>   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>

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)((1ULL << result_bit_width) - 1L), hi);

It seems weird having `- 1L` mixed with ULL now.  It might be better to use right_n_bits_typed<jlong>() here and at line 276.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28062#discussion_r2480470937