JDK-8210434 : [Graal] 8209301 prevents GitHub Graal from compiling with latest JDK
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 12
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-09-05
  • Updated: 2019-08-15
  • Resolved: 2018-09-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 12
12 b11Fixed
Related Reports
Blocks :  
JDK-8208100 - compiler/graalunit/JttLangMathALTest.java times out intermittently on Linux-X64
Blocks :  
JDK-8210478 - Update Graal
Relates :  
JDK-8209301 - JVM rename is_anonymous, host_klass to unsafe specific terminology ahead of Unsafe.defineAnonymousClass deprecation
Description
8209301 which renamed is_anonymous and host_klass to unsafe versions prevents GitHub Graal from compiling with latest JDK.
Comments
isUnsafeAnonymous is the permanent name going forward, we don't anticipate further name changes when Unsafe.defineAnonymousClass is deprecated. It is more likely that isUnsafeAnonymous will be removed all together at the point of deprecation. Another reason as well to avoid introducing names such as "isHosted" or "isNameless" is due to upcoming future work to support dynamic nestmates via a new Lookup.defineClass, see JDK-8171335. Terminology to describe the replacement to Unsafe.defineAnonymousClass is still pending but current thought is a non "findable" class. So, I would advocate either changing the name in Graal or introducing "public boolean isAnonymous() { return isUnsafeAnonymous(); } " temporarily.
10-09-2018
ILW = Graal does not compile with latest JDK, with GitHub Graal, use different JDK version = HMM = P2
10-09-2018
OK, I forgot that. Thanks Doug.
10-09-2018
It will always work with JVMCI8 as that particular API call is only used from jaotc which is a versioned project requiring[1] JDK11. That's why adding back isAnonymous is only required to make it keep working with JDK11. [1] https://github.com/oracle/graal/blob/master/compiler/mx.compiler/suite.py#L1310
09-09-2018
The jaotc/JVMCI rename is in JDK12. Now that jdk.tools.jaotc is in GitHub Graal, if we change jaotc in GitHub Graal to call isUnsafeAnonymous, then it will only be compatible with JDK12 JVMCI, even if we add back isAnonymous. It won't work with JDK11 JVMCI or JDK8 JVMCI, right?
09-09-2018
By "pushing rename upstream", I assume you mean making a GitHub Graal change. The problem with that is not for JDK8 JVMCI but for building GitHub Graal against JDK11 JVMCI. As such, either a revert or re-adding isAnonymous to redirect to new isUnsafeAnonymous would work.
08-09-2018
I think reverting in JDK is better than pushing rename upstream, which would make upstream Graal incompatible with JDK8 JVMCI, right? [~dnsimon] [~mchung]
07-09-2018
I think this suggested fix is good. Alternatively (as I added in JDK-8208100) jdk.vm.ci.hotspot is not exported from module-info.java. However, jdk.vm.ci.hotspot is exported to jdk.internal.vm.compiler and jdk.tools.jatoc via --add-exports at build time. So the rename from isAnonymous to isUnsafeAnonymous is incompatible change that should be coordinated with the upstream Graal project. It's our oversight. We can revert this specific method name to keep the API unchanged for now, i.e. HotSpotResolvedObjectType::isAnonymous, until the dynamic nestmate work determines any API change if appropriate. For dynamic nestmates, JVM CI may need to be aware of the non-findable class (TBD the terminology) I'm fine either way.
07-09-2018
This part of the JVMCI API is (currently) only used by jaotc so I don't have a strong opinion on the permanent naming choice. That said, re-adding `public boolean isAnonymous() { return isUnsafeAnonymous(); }` sounds reasonable to me.
07-09-2018
Yes.
07-09-2018
This is a prerequisite for the next Graal update, right?
07-09-2018
[~lfoltan] [~dnsimon] I have a feeling that the names will change yet again when Unsafe.defineAnonymousClass() is deprecated. It would be nice to have a permanent name. Perhaps isHosted()? isNameless()? We could also add back public boolean isAnonymous() { return isUnsafeAnonymous(); } for backwards compatibility.
07-09-2018