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.

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

(0.41) AArch64: Add space for outgoing JNI argument to J9CInterpreterStackFrame #18484

Merged

Conversation

Akira1Saitoh
Copy link
Contributor

@Akira1Saitoh Akira1Saitoh commented Nov 20, 2023

This commit adds outgoingArguments array to J9CInterpreterStackFrame for AArch64 VM to avoid overwriting preservedGPRs and preservedFPRs when calling out to jitReleaseVMAccess helper from JNI invocation sequence.

This commit also updates ARM64JNILinkage to store JNI method arguments that are passed by stack to the area where the outgoingArguments array resides.

Adding outgoingArguments makes the size of J9CInterpreterStackFrame large and some slots are not accessible with ldp/stp instructions using sp as a base register. Thus, this commit also updates m4 macros in arm64helpers.m4. They use the start of slots for saving JIT GPRs as the base address and x16 as the base register.

Issue: #18149, #18480

Master PR: #18227

…StackFrame

This commit adds outgoingArguments array to J9CInterpreterStackFrame
for AArch64 VM to avoid overwriting preservedGPRs and preservedFPRs when
calling out to jitReleaseVMAccess helper from JNI invocation sequence.

This commit also updates ARM64JNILinkage to store JNI method arguments
that are passed by stack to the area where the outgoingArguments array resides.

Adding outgoingArguments makes the size of J9CInterpreterStackFrame large
and some slots are not accessible with ldp/stp instructions
using sp as a base register. Thus, this commit also updates m4 macros
in arm64helpers.m4. They use the start of slots for saving JIT GPRs
as the base address and x16 as the base register.

Issue: eclipse-openj9#18149

Signed-off-by: Akira Saitoh <saiaki@jp.ibm.com>
@Akira1Saitoh
Copy link
Contributor Author

The original PR (#18227) was merged on October 5th and it did not cause any failures in test cycles.
I am not sure if the fix to 0.41 is still acceptable, but this PR should resolve #18480, which has relatively high reproducible rate (6/50). @pshipton @knn-k @0xdaryl

@pshipton pshipton marked this pull request as draft November 20, 2023 13:01
@pshipton
Copy link
Member

What about #18233, does this also need to be included?

@pshipton
Copy link
Member

I guess that's a side affect of #18212, which showed up in the head stream testing but doesn't directly impact this fix.

@pshipton pshipton marked this pull request as ready for review November 20, 2023 16:55
@pshipton pshipton merged commit 461bf3c into eclipse-openj9:v0.41.0-release Nov 20, 2023
2 checks passed
@Akira1Saitoh
Copy link
Contributor Author

What about #18233, does this also need to be included?

No. #18233 fixes a memory write permission problem introduced by #18212, which is not included in 0.41.

@Akira1Saitoh Akira1Saitoh deleted the aarch64CInterpFrame041 branch November 21, 2023 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants