-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[GR-41977] Adapt to JVMCI API changes. #5455
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
Conversation
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.
Hello @dougxc @christianwimmer, are there any plans to backport the corresponding JVMCI changes to OpenJDK 17 (to my understanding no)? cc @jerboaa
If not, could we please use a more flexible approach to avoid breaking compatibility with JDK 17?
} catch (ReflectiveOperationException ex) { | ||
throw rethrow(ex.getCause()); | ||
} | ||
return runtime().getMirror(method); |
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.
This results in:
/home/runner/work/mandrel/mandrel/mandrel/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotSnippetReflectionProvider.java:134: error: incompatible types: ResolvedJavaMethod cannot be converted to ResolvedJavaType
return runtime().getMirror(method);
When building with JDK 17 (both OpenJDK and LabsJDK).
See https://github.com/graalvm/mandrel/actions/runs/3499526212/jobs/5861191268#step:8:445 and https://github.com/graalvm/mandrel/actions/runs/3499602031/jobs/5861349246#step:7:330, respectively.
@SuppressWarnings({"unchecked"}) | ||
private static <E extends Throwable> RuntimeException rethrow(Throwable ex) throws E { | ||
throw (E) ex; | ||
return runtime().getMirror(field); |
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.
This results in:
/home/runner/work/mandrel/mandrel/mandrel/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotSnippetReflectionProvider.java:142: error: incompatible types: ResolvedJavaField cannot be converted to ResolvedJavaType
return runtime().getMirror(field);
When building with JDK 17 (both OpenJDK and LabsJDK).
See https://github.com/graalvm/mandrel/actions/runs/3499526212/jobs/5861191268#step:8:445 and https://github.com/graalvm/mandrel/actions/runs/3499602031/jobs/5861349246#step:7:330, respectively.
Thanks for the heads-up. Looking into it. |
The current plan is that nothing from the current master branch will be released anymore based on JDK 17. We are switching to "release new GraalVM versions only with the latest Java version", i.e., GraalVM 23.0 will be based (and only based) on Java 20. But in case that plan changes, or you want to release something based on JDK 17, there are only a few small OpenJDK issues that you need to backport into OpenJDK 17: |
Thanks for this info, @christianwimmer. This confirms what I had found out meanwhile. JDK-8296958, JDK-8296961 and JDK-8296960 were on my list to backport to JDK 17u. I'll see to get JDK-8296956 and JDK-8296967 included as well. |
FWIW, all of these have been either backported to the |
@jerboaa it looks like all the listed patches have been backported and will be part of 17.0.6. Is that correct? Can this PR be integrated? FWIW Mandrel seems to build fine with this patch and OpenJDK 17.0.6-beta+6-202212140702 (see https://github.com/graalvm/mandrel/actions/runs/3702797693/jobs/6273484993) |
Correct. |
Does this still need to be backported to JDK 17? |
AFAIK, all changes are in OpenJDK |
Ok good, I thought this PR is about adapting JVMCI API changes in GraalVM and that needs to be backported to work with OpenJDK 17.0.6 or later. |
No description provided.