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

[Christian Thalinger]

> On Sep 17, 2015, at 6:01 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
> 
> If this comes only from adlc, you could fix for adlc only by adding
> 
> WARNINGS_ARE_ERRORS += -Wno-undefined-bool-conversion
> 
> to make/linux/makefiles/adlc.make instead of make/linux/makefiles/gcc.make
> 
> I do however not understand why this could happen during jvmci
> development if you didn't touch any adlc file. Maybe you used another
> compiler (newer/older than 4.8.2) during jvmci development ?

graal-jvmci-8 is still on 8u45.

> 
> Regards,
> Volker
> 
> On Thu, Sep 17, 2015 at 3:57 PM, Gilles Duboscq
> <gilles.m.duboscq at oracle.com> wrote:
>> This warning (undefined-bool-conversion) comes from existing code, not
>> from new code contained in this patch.
>> The warning appears is in adlc:
>> /src/share/vm/adlc/filebuff.cpp:109:8: error: 'this' pointer cannot be
>> null in well-defined C++ code; pointer may be assumed to always convert
>> to true [-Werror,-Wundefined-bool-conversion]
>> We added -Wundefined-bool-conversion during jvmci development because it
>> was making our builds fail but it's not related to jvmci at all.
>> 
>> Gilles
>> 
>> On 17/09/15 09:24, Magnus Ihse Bursie wrote:
>>> 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
>>>