8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call #25765

RealFYang · 2025-06-12T01:54:34Z

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:

Tier1-3 on linux-x64 (release & fastdebug)
Tier1-3 on linux-aarch64 (release & fastdebug)
Tier1-3 on linux-riscv64 (release)
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

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call (Bug - P4)

Reviewers

Tobias Hartmann (@TobiHartmann - Reviewer)
Vladimir Kozlov (@vnkozlov - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25765/head:pull/25765
$ git checkout pull/25765

Update a local copy of the PR:
$ git checkout pull/25765
$ git pull https://git.openjdk.org/jdk.git pull/25765/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25765

View PR using the GUI difftool:
$ git pr show -t 25765

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25765.diff

Using Webrev

Link to Webrev Comment

…g arraycopy runtime call

bridgekeeper · 2025-06-12T01:55:16Z

👋 Welcome back fyang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-06-12T01:55:38Z

@RealFYang This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call

Reviewed-by: thartmann, kvn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 15 new commits pushed to the master branch:

c6ab63d: 8359735: [Ubuntu 25.10] java/lang/ProcessBuilder/Basic.java, java/lang/ProcessHandle/InfoTest.java fail due to rust-coreutils
ff54a64: 8360049: CodeInvalidationReasonTest.java fails with ZGC on AArch64
c11f36e: 8356708: C2: loop strip mining expansion doesn't take sunk stores into account
... and 12 more: https://git.openjdk.org/jdk/compare/08b1fa4cb39e56497052e3ee13e679c8734cf7c5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-06-12T01:56:02Z

@RealFYang The following labels will be automatically applied to this pull request:

graal
hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

RealFYang · 2025-06-12T01:57:16Z

/label remove graal

openjdk · 2025-06-12T01:57:59Z

@RealFYang
The graal label was successfully removed.

mlbridge · 2025-06-12T02:10:11Z

Webrevs

TobiHartmann

Would it be possible to add an IR Framework test for this, checking that the right stub is selected?

RealFYang · 2025-06-13T03:09:42Z

Would it be possible to add an IR Framework test for this, checking that the right stub is selected?

Hi Tobias,
Thanks for taking a look!
Here in this case, StubRoutines::select_arraycopy_function returns an entry address of the runtime call. And this address is feeded to GraphKit::make_runtime_call or IdealKit::make_leaf_call_no_fp to make a runtime call node. So with this change the only difference lies in the target address of the runtime call node. I am not sure, but I guess that it is not supported by the IR framework to detect such a difference for now? Please let me know if I missed anything.

TobiHartmann · 2025-06-13T08:26:45Z

But StubRoutines::select_arraycopy_function also sets copyfunc_name which is printed for the CallNode and should therefore be matchable by the IR framework, right?

RealFYang · 2025-06-16T03:43:27Z

But StubRoutines::select_arraycopy_function also sets copyfunc_name which is printed for the CallNode and should therefore be matchable by the IR framework, right?

Yes. I missed that StubRoutines::select_arraycopy_function would return a copyfunc_name. And I haved added a test to cover the changes for all three callsites. Verified on three different platforms (x64, aarch64 and riscv64). The test passes with the changes and fails otherwise with fastdebug build. Please take another look. Thanks for the suggestion!

TobiHartmann

Thanks for adding a test! I'll run some testing on our side and report back once it passed.

TobiHartmann · 2025-06-18T11:04:41Z

compiler/c2/irTests/stringopts/TestArrayCopySelect.java fails on various platforms with -ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation:

Scenario #0
Compilations (2) of Failed Methods (2)
--------------------------------------
1) Compilation of "static char[] compiler.c2.irTests.stringopts.TestArrayCopySelect.testStrUGetCharsAligned(java.lang.String)":
> Phase "PrintIdeal":
AFTER: print_ideal
  0  Root  === 0 35 65 66  [[ 0 1 3 22 30 ]] inner 
  1  Con  === 0  [[ ]]  #top
  3  Start  === 3 0  [[ 3 5 6 7 8 9 10 ]]  #{0:control, 1:abIO, 2:memory, 3:rawptr:BotPTR, 4:return_address, 5:java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact *}
  5  Parm  === 3  [[ 27 ]] Control !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:-1 (line 71)
  6  Parm  === 3  [[ 31 47 ]] I_O !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:-1 (line 71)
  7  Parm  === 3  [[ 47 31 ]] Memory  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:-1 (line 71)
  8  Parm  === 3  [[ 66 65 47 31 35 ]] FramePtr !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:-1 (line 71)
  9  Parm  === 3  [[ 66 65 31 ]] ReturnAdr !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:-1 (line 71)
 10  Parm  === 3  [[ 23 47 ]] Parm0: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact *  Oop:java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:-1 (line 71)
 22  ConP  === 0  [[ 23 31 ]]  #null
 23  CmpP  === _ 10 22  [[ 24 ]]  !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 24  Bool  === _ 23  [[ 27 ]] [ne] !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 27  If  === 5 24  [[ 28 29 ]] P=0.999999, C=-1.000000 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 28  IfTrue  === 27  [[ 47 ]] #1 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 29  IfFalse  === 27  [[ 31 ]] #0 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 30  ConI  === 0  [[ 31 ]]  #int:-10
 31  CallStaticJava  === 29 6 7 8 9 (30 1 22 ) [[ 32 ]] # Static uncommon_trap(reason='null_check' action='maybe_recompile' debug_id='0')  void ( int ) C=0.000100 TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71) !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 32  Proj  === 31  [[ 35 ]] #0 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 35  Halt  === 32 1 1 8 1  [[ 0 ]]  !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 47  CallStaticJava  === 28 6 7 8 1 (10 1 ) [[ 48 49 50 52 ]] # Static  java.lang.String::toCharArray char[int:>=0] (java/lang/Cloneable,java/io/Serializable):exact * ( java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):NotNull:exact * ) TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71) !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 48  Proj  === 47  [[ 54 ]] #0 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 49  Proj  === 47  [[ 66 54 65 59 ]] #1 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 50  Proj  === 47  [[ 66 65 ]] #2  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 52  Proj  === 47  [[ 65 ]] #5 !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 54  Catch  === 48 49  [[ 55 56 ]]  !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 55  CatchProj  === 54  [[ 65 ]] #0@bci -1  !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 56  CatchProj  === 54  [[ 66 59 ]] #1@bci -1  !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 59  CreateEx  === 56 49  [[ 66 ]]  #java/lang/Throwable (java/io/Serializable):NotNull *  Oop:java/lang/Throwable (java/io/Serializable):NotNull * !jvms: TestArrayCopySelect::testStrUGetCharsAligned @ bci:1 (line 71)
 65  Return  === 55 49 50 8 9 returns 52  [[ 0 ]] 
 66  Rethrow  === 56 49 50 8 9 exception 59  [[ 0 ]] 

2) Compilation of "static java.lang.String compiler.c2.irTests.stringopts.TestArrayCopySelect.testStrUtoBytesAligned(char[])":
> Phase "PrintIdeal":
AFTER: print_ideal
  0  Root  === 0 37 38  [[ 0 1 3 ]] inner 
  1  Con  === 0  [[ ]]  #top
  3  Start  === 3 0  [[ 3 5 6 7 8 9 10 ]]  #{0:control, 1:abIO, 2:memory, 3:rawptr:BotPTR, 4:return_address, 5:char[int:>=0] (java/lang/Cloneable,java/io/Serializable):exact *}
  5  Parm  === 3  [[ 22 ]] Control !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:-1 (line 87)
  6  Parm  === 3  [[ 22 ]] I_O !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:-1 (line 87)
  7  Parm  === 3  [[ 22 ]] Memory  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:-1 (line 87)
  8  Parm  === 3  [[ 38 37 22 ]] FramePtr !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:-1 (line 87)
  9  Parm  === 3  [[ 38 37 ]] ReturnAdr !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:-1 (line 87)
 10  Parm  === 3  [[ 22 ]] Parm0: char[int:>=0] (java/lang/Cloneable,java/io/Serializable):exact * !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:-1 (line 87)
 22  CallStaticJava  === 5 6 7 8 1 (10 1 ) [[ 23 24 25 27 ]] # Static  java.lang.String::valueOf java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * ( char[int:>=0] (java/lang/Cloneable,java/io/Serializable):exact * ) TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87) !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 23  Proj  === 22  [[ 29 ]] #0 !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 24  Proj  === 22  [[ 38 29 37 34 ]] #1 !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 25  Proj  === 22  [[ 38 37 ]] #2  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 27  Proj  === 22  [[ 37 ]] #5  Oop:java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 29  Catch  === 23 24  [[ 30 31 ]]  !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 30  CatchProj  === 29  [[ 37 ]] #0@bci -1  !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 31  CatchProj  === 29  [[ 38 34 ]] #1@bci -1  !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 34  CreateEx  === 31 24  [[ 38 ]]  #java/lang/Throwable (java/io/Serializable):NotNull *  Oop:java/lang/Throwable (java/io/Serializable):NotNull * !jvms: TestArrayCopySelect::testStrUtoBytesAligned @ bci:1 (line 87)
 37  Return  === 30 24 25 8 9 returns 27  [[ 0 ]] 
 38  Rethrow  === 31 24 25 8 9 exception 34  [[ 0 ]] 

Failed IR Rules (2) of Methods (2)
----------------------------------
1) Method "static char[] compiler.c2.irTests.stringopts.TestArrayCopySelect.testStrUGetCharsAligned(java.lang.String)" - [Failed IR rules: 1]:
   * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#C#CALL_OF#_", "arrayof_jshort_disjoint_arraycopy", ">0"}, applyIfPlatform={}, applyIfPlatformOr={}, failOn={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={"UseCompactObjectHeaders", "false"}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
     > Phase "PrintIdeal":
       - counts: Graph contains wrong number of nodes:
         * Constraint 1: "(\\d+(\\s){2}(Call.*.*)+(\\s){2}===.*arrayof_jshort_disjoint_arraycopy )"
           - Failed comparison: [found] 0 > 0 [given]
           - No nodes matched!

2) Method "static java.lang.String compiler.c2.irTests.stringopts.TestArrayCopySelect.testStrUtoBytesAligned(char[])" - [Failed IR rules: 1]:
   * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#C#CALL_OF#_", "arrayof_jshort_disjoint_arraycopy", ">0"}, applyIfPlatform={}, applyIfPlatformOr={}, failOn={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={"UseCompactObjectHeaders", "false"}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
     > Phase "PrintIdeal":
       - counts: Graph contains wrong number of nodes:
         * Constraint 1: "(\\d+(\\s){2}(Call.*.*)+(\\s){2}===.*arrayof_jshort_disjoint_arraycopy )"
           - Failed comparison: [found] 0 > 0 [given]
           - No nodes matched!

RealFYang · 2025-06-19T07:16:33Z

compiler/c2/irTests/stringopts/TestArrayCopySelect.java fails on various platforms with -ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation:

Hi Tobias, I improved the test a bit and added some warmup to it. Now I see it is passing on three different platforms with these extra VM options. Could you please give it another try? Thanks.

test/hotspot/jtreg/compiler/c2/irTests/stringopts/TestArrayCopySelect.java

TobiHartmann

Testing passed. This looks good to me but needs a second review.

RealFYang · 2025-06-20T11:00:16Z

Testing passed. This looks good to me but needs a second review.

Thanks for the review and suggestions. Sure, I'll wait for another review.

vnkozlov

Good.

RealFYang · 2025-06-23T00:25:23Z

Good.

Thanks for the review!

RealFYang · 2025-06-23T00:32:52Z

/integrate

openjdk · 2025-06-23T00:33:38Z

Going to push as commit 6b43939.
Since your change was applied there have been 19 commits pushed to the master branch:

81985d4: 8358526: Clarify behavior of java.awt.HeadlessException constructed with no-args
d627282: 8360143: ProblemList runtime/NMT/VirtualAllocTestType.java
96f71a9: 8255082: HotSpot Style Guide should permit noexcept
... and 16 more: https://git.openjdk.org/jdk/compare/08b1fa4cb39e56497052e3ee13e679c8734cf7c5...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-06-23T00:33:50Z

@RealFYang Pushed as commit 6b43939.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8359270: C2: alignment check should consider base offset when emittin…

6221843

…g arraycopy runtime call

openjdk bot added graal graal-dev@openjdk.org hotspot-compiler hotspot-compiler-dev@openjdk.org labels Jun 12, 2025

openjdk bot removed the graal graal-dev@openjdk.org label Jun 12, 2025

RealFYang marked this pull request as ready for review June 12, 2025 02:00

openjdk bot added the rfr Pull request is ready for review label Jun 12, 2025

TobiHartmann reviewed Jun 12, 2025

View reviewed changes

RealFYang added 2 commits June 16, 2025 11:30

Merge remote-tracking branch 'upstream/master' into JDK-8359270

54fd01b

add test

581f587

TobiHartmann reviewed Jun 18, 2025

View reviewed changes

RealFYang added 2 commits June 19, 2025 15:04

Merge remote-tracking branch 'upstream/master' into JDK-8359270

19271b2

improve test

9dbc4ae

TobiHartmann reviewed Jun 19, 2025

View reviewed changes

test/hotspot/jtreg/compiler/c2/irTests/stringopts/TestArrayCopySelect.java Outdated Show resolved Hide resolved

review

29ef3b2

TobiHartmann approved these changes Jun 20, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 20, 2025

vnkozlov approved these changes Jun 20, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jun 23, 2025

openjdk bot closed this Jun 23, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 23, 2025

8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call #25765

8359270: C2: alignment check should consider base offset when emitting arraycopy runtime call #25765

Uh oh!

Conversation

RealFYang commented Jun 12, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jun 12, 2025

Uh oh!

openjdk bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jun 12, 2025

Uh oh!

RealFYang commented Jun 12, 2025

Uh oh!

openjdk bot commented Jun 12, 2025

Uh oh!

mlbridge bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

RealFYang commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiHartmann commented Jun 13, 2025

Uh oh!

RealFYang commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

TobiHartmann commented Jun 18, 2025

Uh oh!

RealFYang commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

RealFYang commented Jun 20, 2025

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

RealFYang commented Jun 23, 2025

Uh oh!

RealFYang commented Jun 23, 2025

Uh oh!

openjdk bot commented Jun 23, 2025

Uh oh!

openjdk bot commented Jun 23, 2025

Uh oh!

Uh oh!

RealFYang commented Jun 12, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jun 12, 2025 •

edited

Loading

mlbridge bot commented Jun 12, 2025 •

edited

Loading

RealFYang commented Jun 13, 2025 •

edited

Loading

RealFYang commented Jun 16, 2025 •

edited

Loading

RealFYang commented Jun 19, 2025 •

edited

Loading