Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AArch64: Use stp/ldp instructions for preserved registers #17192

Merged
merged 1 commit into from Apr 27, 2023
Merged

AArch64: Use stp/ldp instructions for preserved registers #17192

merged 1 commit into from Apr 27, 2023

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Apr 14, 2023 •
edited

This commit changes code generation for method prologue and epilogue to use the register pair instructions (stp and ldp) for saving and restoring preserved registers when applicable.

@knn-k
Copy link
Contributor Author

knn-k commented Apr 14, 2023

Jenkins test sanity.functional,extended.functional alinux64,amac jdk17

@knn-k
Copy link
Contributor Author

knn-k commented Apr 14, 2023

This commit changes the instructions in the method prologue like this. It does the similar in the method epilogue.

  strimmx w24, [w20, 0]
  strimmx w25, [w20, 8]
  strimmx w26, [w20, 16]
  strimmx w27, [w20, 24]
  strimmx w28, [w20, 32]
    ↓
  stpoffx w24, w25, [w20, 0]
  stpoffx w26, w27, [w20, 16]
  strimmx w28, [w20, 32]

@knn-k knn-k marked this pull request as ready for review April 14, 2023 11:27
@knn-k knn-k requested a review from 0xdaryl April 14, 2023 11:56
@knn-k
Copy link
Contributor Author

knn-k commented Apr 17, 2023

Jenkins test sanity.functional,extended.functional alinux64,amac jdk17

@knn-k
Copy link
Contributor Author

knn-k commented Apr 17, 2023

Jenkins test sanity.functional alinux64 jdk17

@knn-k
Copy link
Contributor Author

knn-k commented Apr 17, 2023

The failure in cmdLineTest_J9test_common_0 on AArch64 Linux looks like #13577.
https://openj9-jenkins.osuosl.org/job/Test_openjdk17_j9_sanity.functional_aarch64_linux_Personal/221/

Testing: -Xlockword minimizeFootprint mode
Test start time: 2023/04/17 04:05:50 Coordinated Universal Time
Running command: "/home/jenkins/workspace/Grinder/openjdkbinary/j2sdk-image/bin/java"   -Xdump -cp "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../../jvmtest/functional/cmdLineTests/utils/utils.jar" -Xlockword:mode=minimizeFootprint VMBench.FibBench
Time spent starting: 4 milliseconds
Time spent executing: 2276 milliseconds
Test result: FAILED
Output from test:
 [OUT] Fibonacci: iterations = 10000
 [OUT] fibonacci(12) = 144
 [ERR] Unhandled exception
 [ERR] Type=Segmentation error vmState=0x00000000
 [ERR] J9Generic_Signal_Number=00000018 Signal_Number=0000000b Error_Value=00000000 Signal_Code=00000001
 [ERR] Handler1=0000FFFFAB5F2484 Handler2=0000FFFFAB559AC0 InaccessibleAddress=0000000000000024

0xdaryl
0xdaryl reviewed Apr 17, 2023
View reviewed changes
Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. A couple of minor variable renames requested.

runtime/compiler/aarch64/codegen/ARM64PrivateLinkage.cpp Outdated
@@ -783,17 +783,45 @@ void J9::ARM64::PrivateLinkage::createPrologue(TR::Instruction *cursor)

int32_t preservedRegisterOffsetFromJavaSP = outgoingArgsSize;

TR::RealRegister *regToStore = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: can you rename this to pendingRegToStore for improved readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed as suggested.

runtime/compiler/aarch64/codegen/ARM64PrivateLinkage.cpp Outdated
@@ -881,16 +909,42 @@ void J9::ARM64::PrivateLinkage::createEpilogue(TR::Instruction *cursor)
int32_t preservedRegisterOffsetFromJavaSP = cg()->getLargestOutgoingArgSize() + getOffsetToFirstParm(); // outgoingArgsSize
TR::RealRegister::RegNum firstPreservedGPR = TR::RealRegister::x21;
TR::RealRegister::RegNum lastPreservedGPR = TR::RealRegister::x28;
TR::RealRegister *regToLoad = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: can you rename this to pendingRegToLoad for improved readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed as suggested.

@knn-k
AArch64: Use stp/ldp instructions for preserved registers
2ed4ecc 
This commit changes code generation for method prologue and epilogue
to use the register pair instructions (stp and ldp) for saving and
restoring preserved registers when applicable.

Signed-off-by: KONNO Kazuhiro <konno@jp.ibm.com>
@knn-k
Copy link
Contributor Author

knn-k commented Apr 17, 2023

Jenkins test sanity.functional alinux64,amac jdk17

@0xdaryl 0xdaryl self-assigned this Apr 27, 2023
@0xdaryl 0xdaryl merged commit 229feb8 into eclipse-openj9:master Apr 27, 2023
7 checks passed
@knn-k knn-k deleted the aarch64useStpLdp branch April 27, 2023 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xdaryl 0xdaryl 0xdaryl approved these changes

Assignees

@0xdaryl 0xdaryl

Labels
arch:aarch64 comp:jit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@knn-k @0xdaryl