RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Magnus Ihse Bursie]

On 2015-09-14 09:24, Christian Thalinger wrote:
> The JEP itself can be found here:
>
> https://bugs.openjdk.java.net/browse/JDK-8062493 <https://bugs.openjdk.java.net/browse/JDK-8062493>
>
> Here are the webrevs:
>
> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>
> The change has already undergone a few iterations of internal review by different people of different teams.  Most comments and suggestions were handled accordingly.  Although there is one open item we agreed we will address after the integration of JEP 243 and that work is captured in RFE:
>
> https://bugs.openjdk.java.net/browse/JDK-8134994 <https://bugs.openjdk.java.net/browse/JDK-8134994>
>
> SQE is still working on the tests and all test tasks can be seen as sub-tasks of the JEP.
>
> The integration will happen under the bug number:
>
> https://bugs.openjdk.java.net/browse/JDK-8136421 <https://bugs.openjdk.java.net/browse/JDK-8136421>
>
Hi Christian,

(Adding build-dev since this review includes some major build changes.)

The makefile changes looks good in general to me. I have not reviewed 
the source code changes.

Some comments:

* modules.xml:
Make sure you get sign-off from a Jigsaw developer for modifying this file.

* hotspot/make/linux/makefiles/gcc.make:
Seems unfortunate to have to disable a new warning 
(undefined-bool-conversion) for newly incorporated code. Is it not 
possible to fix the code emitting this warning instead?

* make/common/MakeBase.gmk and hotspot/make/gensrc/Gensrc-java.base.gmk:
I see a coming merge conflict. In jdk9/dev, there is now a new function 
that performs the same function as CreatePath, but it's named PathList. 
(It's a bit unfortunate that this code snippet has bounced around as 
patches without a definite name.) I assume you are going to push this 
through a hotspot forest. If the PathList patch reaches the hotspot repo 
before this, please remove the CreatePath from MakeBase, and change the 
calls to CreatePath to PathList instead. (I could only find such calls 
in hotspot/make/gensrc/Gensrc-java.base.gmk). If this patch goes in 
before that, we'll need to give a heads-up to the integrator about this 
conflict.

Another potential coming merge conflict relates to ListPathsSafely in 
Gensrc-java.base.gmk. There is currenlty a review out from Erik which 
modifies the API for ListPathsSafely. If/when it goes in, the call to 
ListPathsSafely in Gensrc-java.base.gmk will need to be modified (Erik 
can give advice on how). Depending on timing, this too might hit the 
integrator rather than your push.

/Magnus