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

[Tobias Hartmann]

On Thu, 30 Oct 2025 14:51:53 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 go on vacation, and was not able to catch the issue (https://github.com/openj...

Looks good to me. Thank you for your persistence on improving our testing! Good that we caught this in-time for JDK 26.

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

> 677:     @IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = { "bmi2", "true" })
> 678:     public static long test20(int x) {
> 679:         // Analysis of when this used to produce wrong results on Windows:

Suggestion:

        // Analysis of when this is used to produce wrong results on Windows:

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

> 698:         //
> 699:         // But watch out: on windows 1UL is only a 32 bit value. Intended was probably 1ULL.
> 700:         // So when we caluculate "1UL << 32", we just get 1. And so then hi would be 0 now.

Suggestion:

        // So when we calculate "1UL << 32", we just get 1. And so then hi would be 0 now.

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

> 702:         //
> 703:         // We create type [lo, hi]:
> 704:         // windowns: [0, 0]           -> constant zero

Suggestion:

        // Windows: [0, 0]           -> constant zero

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

> 726:     @IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = { "bmi2", "true" })
> 727:     public static long test21(long x) {
> 728:         // Analysis of when this used to produce wrong results on Windows:

Suggestion:

        // Analysis of when this is used to produce wrong results on Windows:

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28062#pullrequestreview-3402858715
PR Review Comment: https://git.openjdk.org/jdk/pull/28062#discussion_r2480368185
PR Review Comment: https://git.openjdk.org/jdk/pull/28062#discussion_r2480369611
PR Review Comment: https://git.openjdk.org/jdk/pull/28062#discussion_r2480369974
PR Review Comment: https://git.openjdk.org/jdk/pull/28062#discussion_r2480368555