-
Notifications
You must be signed in to change notification settings - Fork 6.2k
simplify unsafe intrinsic processing #28940
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
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back jrose! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
Status: No testing beyond looking for smoke when I run javac (in the changed VM). I have not gone looking for inlining failures. I know they are endemic in C2. I think the interpreter support (native methods) is stable, as are JDK code adjustments. The vmIntrinsics.hpp declarations define a bunch of “polymorphic intrinsic wrappers” (F_PW). C2 support is not started yet. It should be relatively simple, as with C1. The C1 support is nearly stable. Smoke test with javac passes. I added some C1 canonicalization logic for back-to-back conversions. |
|
Overall goal is to thin out the current Dark Forest of intrinsics, which has become overgrown. |
ExE-Boss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the final modifier should be kept on the Unsafe methods to ease the burden on reviewing the git diff.
|
|
||
| /** Special-access version of {@link #getInt(Object, long)} */ | ||
| @ForceInline | ||
| public int getIntMO(byte memoryOrder, Object o, long offset) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to keep these parametrised memory order implementation methods private without changing the Java API by changing @IntrinsicCandidate to @ForceInline on all the [get/put/compareAnd[Set/Exchange]]<Type>[/Opaque/Acquire/Release/Volatile] methods.
first cut: passes smoke tests in interpreter and C1 (aarch64)
https://bugs.openjdk.org/browse/JDK-8374169
too many intrinsics
Fold getIntAcquire and every other getFooBar into getPrimitiveBitsMO(mo, bt, base, offset).
Do similar moves for put and the various CAS operations.
Progress
Reviewers without OpenJDK IDs
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28940/head:pull/28940$ git checkout pull/28940Update a local copy of the PR:
$ git checkout pull/28940$ git pull https://git.openjdk.org/jdk.git pull/28940/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28940View PR using the GUI difftool:
$ git pr show -t 28940Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28940.diff