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

[Christian Thalinger]

> On Sep 17, 2015, at 3:57 AM, 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]

I think all these got fixed in 9.  In particular the code you are referring to was removed.  Could you try to compile graal-jvmci-9 without the warning disabled?

> 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
>>