RFR: 8369902: C2 SuperWord: wrong result because filterin NaN instead of zero in MemPointerParser::canonicalize_raw_summands
    Emanuel Peter 
    epeter at openjdk.org
       
    Fri Oct 17 08:44:45 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-8365985). I now added some extra comments to the test it self, in the "future work section".
--------------------------------
**More Background Info**
I've been asked this a few times now. "Where do the NaN summands get detected eventually?". So let me explain the pipeline:
- `MemPointerParser::parse` creates a `MemPointer`. A `MemPointer` cannot be (in)valid. Rather, it can either be "trivial" `1 * pointer`, or non-trivial (some actual decomposition with multiple summands).
  - We create the raw summands, that may contain `NaN` or `zero`.
  - `canonicalize_raw_summands`: sorts and combines raw summands. Zero summands need to be filtered out.
  - `create_summands`: turn raw summands into (regular) summands. The only "surprise" that could happen here is that we have an overflow on `_con` addition, leading to a `NaN`.
  - `canonicalize_summands`: sort and combine summands. When adding scales, we could introduce `NaN` again. Zero summands are filtered out again.
  - Call `MemPointer::make` to create a summand. We could now have `_con = NaN` or a scale that is `NaN`.
  - `MemPointer::make`: checks `has_no_NaN_in_con_and_summands`, and if that fails, we just create a trivial `MemPointer`, which is alway correct, but ultimately not helpful for optimizations, so probably no vectorization from that happens.
I hope that helps :)
-------------------------------------
**More thoughts**
I had always thought that large constant offsets are very rare. And I still think so. But the fuzzer case shows that long constants could arise from IGVN optimizations as well.
If you think that long-constants in pointers are important: file an RFE, and show me some use-cases that are important for performance.
-------------
Commit messages:
 - add comments to TestAliasingFuzzer.java
 - typo
 - add fuzzer test
 - test improvements and fix
 - second test
 - rename test
 - JDK-8369902
Changes: https://git.openjdk.org/jdk/pull/27848/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=27848&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8369902
  Stats: 253 lines in 4 files changed: 251 ins; 0 del; 2 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