RFR: 8369902: C2 SuperWord: wrong result because filterin NaN instead of zero in MemPointerParser::canonicalize_raw_summands [v3]
Emanuel Peter
epeter at openjdk.org
Mon Oct 20 05:59:46 UTC 2025
> **TLDR** `is_NaN` -> `is_zero`, just like the code comment says.
>
> Thanks to @mhaessig for debugging the ARM32 bug below. He found the buggy line of code.
>
> ----------------------------------------
>
> **Details**
>
> It seems there is a little "typo" (logic error) in `MemPointerParser::canonicalize_raw_summands` that slipped through the cracks in https://github.com/openjdk/jdk/pull/24278. The JavaFuzzer now found an example, and independently the issue was also reported on ARM32 [JDK-8368578](https://bugs.openjdk.org/browse/JDK-8368578).
>
> Filtering out `NaN` instead of `zero` for the `scaleL` has two manifestations:
> - If `scaleL` is zero, but does not get filtered out even though it should be: we hit the assert in `MemPointerSummand` constructor, `assert(!_scale.is_zero(), "non-zero scale");`.
> - See [JDK-8368578](https://bugs.openjdk.org/browse/JDK-8368578), though those tests seem to only fail on ARM32, and nowhere else.
> - I was able to construct a `MemorySegment` regression test, see `TestMemorySegmentFilterSummands.test1`. I suspect that the ARM32 failures happened on an array, as it failed in places like `BigInteger::implMultiplyToLen`. But now I was able to reproduce it with native memory, to get a pointer expression that has the same cancellation issue.
> - If `scaleL` is `NaN`, and gets filtered even though it should not be: We get a non-trivial MemPointer that is missing a summand. So we will succeed in optimizing, but with wrong assumptions. We generate a runtime aliasing check that is incorrect, leading to wrong results.
> - This was reported by the fuzzer, see attached `TestDoNotFilterNaNSummands`.
> - I was also able to create a simpler example with `MemorySegments`, see attached `TestMemorySegmentFilterSummands.test2`.
>
> **Why did this slip through the cracks?**
>
> In https://github.com/openjdk/jdk/pull/24278 I added pretty extensive testing, even fuzzer style tests, see `TestAliasingFuzzer.java`. But I think all of those tests exercise `scale` that are in "nice" [int ranges](https://github.com/openjdk/jdk/pull/24278/files#diff-26de03e864a492fe8aa8178818968f2097b99cf36a763605e2fb11fbc04eedffR303-R322). Also the JavaFuzzer does not directly generate such long constants for array accesses (not possible without Unsafe I think), we were lucky that it generated the index with `%` that got optimized to some magic long constant.
>
> There is already an RFE filed for improvements to `TestAliasingFuzzer.java`: [JDK-8365985](https://bugs.openjdk.org/browse/JDK-836...
Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains nine additional commits since the last revision:
- Merge branch 'master' into JDK-8369902-SW-VPointer-NaN-zero
- Update test/hotspot/jtreg/compiler/loopopts/superword/TestMemorySegmentFilterSummands.java
Co-authored-by: Manuel Hässig <manuel at haessig.org>
- add comments to TestAliasingFuzzer.java
- typo
- add fuzzer test
- test improvements and fix
- second test
- rename test
- JDK-8369902
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/27848/files
- new: https://git.openjdk.org/jdk/pull/27848/files/e19a22f7..3aa0dc1a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=27848&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=27848&range=01-02
Stats: 17082 lines in 502 files changed: 8392 ins; 6836 del; 1854 mod
Patch: https://git.openjdk.org/jdk/pull/27848.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/27848/head:pull/27848
PR: https://git.openjdk.org/jdk/pull/27848
More information about the hotspot-compiler-dev
mailing list