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

Volker Simonis volker.simonis at gmail.com
Thu Sep 17 16:01:38 UTC 2015


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 ?

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 ($(USECLANG), 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



More information about the hotspot-dev mailing list