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

[Magnus Ihse Bursie]

On 2015-09-16 18:52, Christian Thalinger wrote:
>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
>>
>> 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.
> I’ve asked Alan to take a look.
>
>> * 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?
> We had this question before.  It’s not obvious because you can’t see it in the regular diff views but this is under:
>
> ifeq ($(USE_CLANG), true)

I'm not sure I understand why that's relevant..? Isn't it just as 
important to try to submit warning-free code when compiling with clang 
as with any other compiler? Or is clang just being anal about the code 
in question?

/Magnus

>
>> * 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.
> Thanks for the heads up.
>
>> 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.
> Ok, thanks.
>
>> /Magnus