RFR: 8351468: C2: array fill optimization assigns wrong type to intrinsic call [v2]
Roberto CastaƱeda Lozano
rcastanedalo at openjdk.org
Wed Mar 12 12:38:36 UTC 2025
> The [array fill optimization](https://github.com/openjdk/jdk/blob/1d147ccb4cfcb1da23664ac941e56ac542a7ac61/src/hotspot/share/opto/loopTransform.cpp#L3533) replaces simple innermost loops that fill an array with copies of the same primitive value:
>
>
> for (int i = 0; i < array.length; i++) {
> array[i] = 0;
> }
>
> with a call to an [array filling intrinsic](https://github.com/openjdk/jdk/blob/1d147ccb4cfcb1da23664ac941e56ac542a7ac61/src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp#L1665) that is specialized for the array element type:
>
>
> arrayof_jint_fill(array, 0, array.length)
>
>
> The optimization retrieves the (basic) array element type from calling `MemNode::memory_type()` on the original filling store. This is incorrect for stores of `short` values, since these are represented by `StoreC` nodes [whose `memory_type()` is `T_CHAR`](https://github.com/openjdk/jdk/blob/1fe45265e446eeca5dc496085928ce20863a3172/src/hotspot/share/opto/memnode.hpp#L680). As a result, the optimization wrongly assigns the address type `char[]` to `short` array fill loops. This can cause miscompilations due to missing anti-dependences, see the [issue description for further detail](https://bugs.openjdk.org/projects/JDK/issues/JDK-8351468).
>
> This changeset proposes retrieving the (basic) array element type from the store address type instead. This ensures that the accurate address type is assigned to the intrinsic call, preventing missed anti-dependences and other potential issues caused by mismatching types. A more general solution to this issue, and a way to prevent similar bugs in the future, would be to define a `StoreS` node returning the appropriate `memory_type()`. I propose to investigate this in a separate RFE and keep this fix as minimal and non-intrusive as possible for backportability.
>
> **Testing:** tier1-5, stress testing (linux-x64, windows-x64, macosx-x64, linux-aarch64, macosx-aarch64).
Roberto CastaƱeda Lozano 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 five additional commits since the last revision:
- Merge master
- Remove temporary assertion
- Add test
- Refine assertion to deal with aliasing byte/Boolean types
- Compute basic type from the store's address type, circumventing the innacurate MemNode::memory_type()
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/24005/files
- new: https://git.openjdk.org/jdk/pull/24005/files/47e478a6..90fd7660
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=24005&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=24005&range=00-01
Stats: 42515 lines in 533 files changed: 19874 ins; 14843 del; 7798 mod
Patch: https://git.openjdk.org/jdk/pull/24005.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/24005/head:pull/24005
PR: https://git.openjdk.org/jdk/pull/24005
More information about the hotspot-compiler-dev
mailing list