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

Accelerate Unsafe CAS Intrinsics on Power and X #7438

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

IBMJimmyk
Copy link
Contributor

@IBMJimmyk IBMJimmyk commented Aug 12, 2024

Adds lbarx, lharx, stbcx_r and sthcx_r instruction support for Power. These instructions are not currently in use but will be needed for future enhancements targetting CAS on byte and short length data.

Updates willBeEvaluatedAsCallByCodeGen to reflect the new acceleration of Unsafe compareAndExchange methods on X.

Edit:
X changes are dependent on this OpenJ9 PR:
eclipse-openj9/openj9#19991
Behavior of inlineCompareAndSwapNative and inlineCompareAndSwapObjectNative in J9TreeEvaluator.cpp must match behavior in willBeEvaluatedAsCallByCodeGen in OMRCodeGenerator.cpp

@IBMJimmyk
Copy link
Contributor Author

@hzongaro The compiler/x/codegen/OMRCodeGenerator.cpp change goes along with the OpenJ9 changes for accelerating Unsafe CAS on X and unfortunately is inside OMR instead of OpenJ9. I also need to double check if the change to callDoesAnImplicitAsyncCheck is correct.

@zlwang I need a review on the Power instructions being added.

Also, would it be preferred if this PR was split into two? I was working on the Power and X accelerations at the same time and the end result just happened to be 2 fairly disjoint changes to OMR.

Copy link
Contributor

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

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

I think the changes for x/codegen/OMRCodeGenerator.cpp and compiler/optimizer/RedundantAsyncCheckRemoval.cpp look correct.

@IBMJimmyk
Copy link
Contributor Author

@zlwang did you get a chance to look over these changes?

Copy link
Contributor

@zl-wang zl-wang left a comment

Choose a reason for hiding this comment

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

LGTM

@hzongaro
Copy link
Contributor

Sorry for the late comment. May I ask you to reformat your commit comment so that lines do not exceed 72 characters in length, per https://github.com/eclipse/omr/blob/master/CONTRIBUTING.md#commit-guidelines?

@hzongaro
Copy link
Contributor

Jenkins build all

@IBMJimmyk IBMJimmyk changed the title WIP: Accelerate Unsafe CAS Intrinsics on Power and X Accelerate Unsafe CAS Intrinsics on Power and X Sep 23, 2024
Adds lbarx, lharx, stbcx_r and sthcx_r instruction support for Power.
These instructions are not currently in use but will be needed for
future enhancements targetting CAS on byte and short length data.

Updates willBeEvaluatedAsCallByCodeGen to reflect the new acceleration
of Unsafe compareAndExchange methods on X.

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

I fixed the commit message.

@hzongaro
Copy link
Contributor

Jenkins build all

@hzongaro hzongaro self-assigned this Sep 23, 2024
@hzongaro
Copy link
Contributor

@IBMJimmyk, there is a failure in the Linux ppc64le testing:

14:39:12  31: �[0;32m[----------] �[m14 tests from Special/PPCDirectEncodingTest
14:39:12  31: free(): invalid next size (normal)
14:39:12  31/31 Test #31: compunittest ......................Child aborted***Exception:   1.72 sec

Is that related to the Binary Encoder test change that is part of this pull request?

@IBMJimmyk
Copy link
Contributor Author

It doesn't seem like it should be. I changed PPCTrg1MemEncodingTest, PPCRecordFormSanityTest and PPCMemSrc1EncodingTest not PPCDirectEncodingTest. Furthermore, the test passed under the AIX build and it should be the same test.

@hzongaro
Copy link
Contributor

It looks like the failure in PPCDirectEncodingTest is due to known issue #6571.

@hzongaro hzongaro merged commit 56a6b83 into eclipse-omr:master Oct 4, 2024
16 of 17 checks passed
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