RFR: 8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call [v3]
Tobias Hartmann
thartmann at openjdk.org
Thu Jun 19 10:53:00 UTC 2025
On Thu, 19 Jun 2025 07:18:49 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Hi, please consider this change fixing alignment check when emitting arraycopy runtime call.
>>
>> There are currently four callsites of `StubRoutines::select_arraycopy_function` in hotspot C2 shared code where we emit arraycopy runtime calls [1-4]. Three of them [2-4] missed base offset when calculation alignment for both source and destination array addresses. Seems they assume a base offset of 8 bytes, which is not always true. Base offset becomes 4 bytes under either `-XX:+UseCompactObjectHeaders` or `-XX:-UseCompressedClassPointers`.
>>
>> And `StubRoutines::select_arraycopy_function` selects the right arraycopy runtime call based on this alignment. As a result, it could see an incorrect `aligned` param about the array addresses and thus a wrong arraycopy runtime call is selected. This is causing performance regressions (like Dacapo Spring) on some linux-riscv64 platforms like Sifive Unmatched or Premier P550 SBCs where misaligned memory access is very slow.
>>
>> Proposed change fixes this issue by taking base offset into account when checking the alignment, which is very similar to [1].
>>
>> Testing:
>> - [x] Tier1-3 on linux-x64 (release & fastdebug)
>> - [x] Tier1-3 on linux-aarch64 (release & fastdebug)
>> - [x] Tier1-3 on linux-riscv64 (release)
>> - [x] Dacapo spring performance test on linux-riscv64 (w/wo `-XX:+UseCompactObjectHeaders` / `-XX:-UseCompressedClassPointers`)
>>
>> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/macroArrayCopy.cpp#L341
>> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L1584
>> [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L1666
>> [4] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/stringopts.cpp#L1481
>
> Fei Yang 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:
>
> - improve test
> - Merge remote-tracking branch 'upstream/master' into JDK-8359270
> - add test
> - Merge remote-tracking branch 'upstream/master' into JDK-8359270
> - 8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call
test/hotspot/jtreg/compiler/c2/irTests/stringopts/TestArrayCopySelect.java line 50:
> 48: public static void main(String[] args) {
> 49: TestFramework.runWithFlags("-XX:+UseCompactObjectHeaders", "-XX:-CompactStrings", "-XX:MaxInlineSize=70", "-XX:MinInlineFrequencyRatio=0");
> 50: TestFramework.runWithFlags("-XX:-UseCompactObjectHeaders", "-XX:-CompactStrings", "-XX:MaxInlineSize=70", "-XX:MinInlineFrequencyRatio=0");
Wouldn't it make more sense to simply enforce inlining via a compile command?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25765#discussion_r2156701312
More information about the hotspot-compiler-dev
mailing list