Integrated: 8351468: C2: array fill optimization assigns wrong type to intrinsic call

[Roberto Castañeda Lozano]

On Wed, 12 Mar 2025 09:47:17 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

> 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. Additionally, the changeset makes it easier to reason about correctness by explicitly disabling the optimization for mismatched stores (where the type of the value to be stored differs from the element type of the destination array). Such stores were not optimized before, but only due to pattern matching limitations.
> 
> Assuming mismatched stores are discarded (as proposed here), an alternative solution would be to define a StoreS node returning the appropriate `memory_type()`. This could be desirable even as a complement to this fix, to prevent similar bugs in the future. I propose to investigate the introduction of a StoreS node in a separate RFE, because it is a much larger and more intrusive changeset, and go with this minimal, local, and non-intrusive fix for backportability.
> 
> **Testing:** tier1-5, stress testing (linux-x64, windows-x64, macosx-x64, linux-aarch64, macosx-aarch64).

This pull request has now been integrated.

Changeset: de580090
Author:    Roberto Castañeda Lozano <rcastanedalo at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/de580090cd9ada313a878975b9f183045d293684
Stats:     489 lines in 3 files changed: 486 ins; 0 del; 3 mod

8351468: C2: array fill optimization assigns wrong type to intrinsic call

Reviewed-by: epeter, thartmann, qamai

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

PR: https://git.openjdk.org/jdk/pull/24005