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

[Magnus Ihse Bursie]

On 2015-09-16 22:25, Christian Thalinger wrote:
>
>> On Sep 16, 2015, at 10:11 AM, Magnus Ihse Bursie 
>> <magnus.ihse.bursie at oracle.com 
>> <mailto:magnus.ihse.bursie at oracle.com>> wrote:
>>
>> 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 
>>>> <mailto: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/%7Ekvn/JVMCI/webrev.top/> 
>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>>>>> <http://cr.openjdk.java.net/%7Ekvn/JVMCI/webrev.top/>>
>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>>>> <http://cr.openjdk.java.net/%7Ekvn/JVMCI/webrev.hotspot/> 
>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>>>> <http://cr.openjdk.java.net/%7Ekvn/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?
>
> I don’t have a Clang compiler at hand but Clang is anal about 
> everything.  Do you want that suppression to be removed?

It's more a hotspot code quality issue, not a build system issue, so I 
won't insist. I just wanted to point out that this change will start 
hiding a new kind of warning for all files in hotspot. Unless there was 
a compelling reason, I would personally rather see an effort to fix the 
code in question. But if no-one from Hotspot agrees on this, I'll drop it.

/Magnus