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

AArch64: Use dmb limitation enum #20188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Spencer-Comin
Copy link
Contributor

This commit replaces the use of magic numbers for the limitation immediate for dmb instructions with the AArch64BarrierLimitation enum.

NOTE: Requires coordinated merge with eclipse/omr#7465

This commit replaces the use of magic numbers for the limitation
immediate for dmb instructions with the AArch64BarrierLimitation enum.

Signed-off-by: Spencer Comin <[email protected]>
@Spencer-Comin
Copy link
Contributor Author

Internal JDK11 sanity.functional testing: https://hyc-runtimes-jenkins.swg-devops.com/job/Pipeline-Build-Test-Personal/23966/

@@ -97,7 +97,7 @@ void J9::ARM64::JNILinkage::releaseVMAccess(TR::Node *callNode, TR::Register *vm
// Arm Architecture Reference Manual states:
// "This architecture assumes that all PEs that use the same operating system or hypervisor are in the same Inner Shareable shareability domain"
// thus, inner shareable dmb suffices
generateSynchronizationInstruction(cg(), TR::InstOpCode::dmb, callNode, 0xb);
generateSynchronizationInstruction(cg(), TR::InstOpCode::dmb, callNode, TR::InstOpCode::ishst);
Copy link
Contributor

Choose a reason for hiding this comment

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

0xb means TR::InstOpCode::ish, while the comments above says dmb ishst.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would a dmb ishst be sufficient for the spin lock implementation here?

Looking at the example spin lock implementation in the Arm Architecture Reference Manual (Appendix K 14.3.1), I wonder if we could replace the dmb with a prfm and relax the ldxrx to a ldaxrx.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, is the dmb necessary given that we are using ldxrx to read the lock?

@knn-k
Copy link
Contributor

knn-k commented Sep 19, 2024

Jenkins test sanity.functional,sanity.system alinux64,amac jdk17 depends eclipse/omr#7465

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.

2 participants