RFR(XS): 8140239: Fix product build after "8132168: Support IdealGraphVisualizer in optimized build"

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Fri Oct 23 19:20:15 UTC 2015


Hi,

Yes, I think that is better, too.
I just wanted to make the quick fix for the build, first.

Thanks for further looking into this!

Best regards,
  Goetz.

From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
Sent: Friday, October 23, 2015 8:40 PM
To: Vladimir Kozlov <vladimir.kozlov at oracle.com>
Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(XS): 8140239: Fix product build after "8132168: Support IdealGraphVisualizer in optimized build"


On Oct 22, 2015, at 5:24 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>> wrote:

On 10/23/15 7:47 AM, Christian Thalinger wrote:

Why are we prefixing flags with CONST_ in product builds?

It was your change:
8024545: make develop and notproduct flag values available in product builds
http://cr.openjdk.java.net/~twisti/8024545/webrev/src/share/vm/runtime/globals.hpp.udiff.html

Damn.  I knew it :-)

I vaguely remember that I did that because of getting the flag address for the flag table.  But this works as well:

diff -r bc48b669bc66 src/share/vm/runtime/globals.cpp
--- a/src/share/vm/runtime/globals.cpp  Mon Oct 19 00:24:58 2015 -0700
+++ b/src/share/vm/runtime/globals.cpp  Fri Oct 23 08:35:34 2015 -1000
@@ -536,7 +536,7 @@ const char* Flag::flag_error_str(Flag::E
 // 4991491 do not "optimize out" the was_set false values: omitting them
 // tickles a Microsoft compiler bug causing flagTable to be malformed

-#define NAME(name) NOT_PRODUCT(&name) PRODUCT_ONLY(&CONST_##name)
+#define NAME(name) (void*)&name

 #define RUNTIME_PRODUCT_FLAG_STRUCT(     type, name, value, doc) { #type, XSTR(name), &name,      NOT_PRODUCT_ARG(doc) Flag::Flags(Flag::DEFAULT | Flag::KIND_PRODUCT) },
 #define RUNTIME_PD_PRODUCT_FLAG_STRUCT(  type, name,        doc) { #type, XSTR(name), &name,      NOT_PRODUCT_ARG(doc) Flag::Flags(Flag::DEFAULT | Flag::KIND_PRODUCT | Flag::KIND_PLATFORM_DEPENDENT) },
diff -r bc48b669bc66 src/share/vm/runtime/globals.hpp
--- a/src/share/vm/runtime/globals.hpp  Mon Oct 19 00:24:58 2015 -0700
+++ b/src/share/vm/runtime/globals.hpp  Fri Oct 23 08:35:34 2015 -1000
@@ -4129,9 +4129,9 @@ public:
 #define DECLARE_MANAGEABLE_FLAG(type, name, value, doc)   extern "C" type name;
 #define DECLARE_PRODUCT_RW_FLAG(type, name, value, doc)   extern "C" type name;
 #ifdef PRODUCT
-#define DECLARE_DEVELOPER_FLAG(type, name, value, doc)    extern "C" type CONST_##name; const type name = value;
-#define DECLARE_PD_DEVELOPER_FLAG(type, name, doc)        extern "C" type CONST_##name; const type name = pd_##name;
-#define DECLARE_NOTPRODUCT_FLAG(type, name, value, doc)   extern "C" type CONST_##name;
+#define DECLARE_DEVELOPER_FLAG(type, name, value, doc)    const type name = value;
+#define DECLARE_PD_DEVELOPER_FLAG(type, name, doc)        const type name = pd_##name;
+#define DECLARE_NOTPRODUCT_FLAG(type, name, value, doc)   const type name = value;
 #else
 #define DECLARE_DEVELOPER_FLAG(type, name, value, doc)    extern "C" type name;
 #define DECLARE_PD_DEVELOPER_FLAG(type, name, doc)        extern "C" type name;
@@ -4152,9 +4152,9 @@ public:
 #define MATERIALIZE_MANAGEABLE_FLAG(type, name, value, doc)   type name = value;
 #define MATERIALIZE_PRODUCT_RW_FLAG(type, name, value, doc)   type name = value;
 #ifdef PRODUCT
-#define MATERIALIZE_DEVELOPER_FLAG(type, name, value, doc)    type CONST_##name = value;
-#define MATERIALIZE_PD_DEVELOPER_FLAG(type, name, doc)        type CONST_##name = pd_##name;
-#define MATERIALIZE_NOTPRODUCT_FLAG(type, name, value, doc)   type CONST_##name = value;
+#define MATERIALIZE_DEVELOPER_FLAG(type, name, value, doc)
+#define MATERIALIZE_PD_DEVELOPER_FLAG(type, name, doc)
+#define MATERIALIZE_NOTPRODUCT_FLAG(type, name, value, doc)
 #else
 #define MATERIALIZE_DEVELOPER_FLAG(type, name, value, doc)    type name = value;
 #define MATERIALIZE_PD_DEVELOPER_FLAG(type, name, doc)        type name = pd_##name;

I think we should do that and not make flags conditional like the fix for this bug.



Vladimir



#ifdef PRODUCT
#define MATERIALIZE_DEVELOPER_FLAG(type, name, value, doc)    type CONST_##name = value;
#define MATERIALIZE_PD_DEVELOPER_FLAG(type, name, doc)        type CONST_##name = pd_##name;
#define MATERIALIZE_NOTPRODUCT_FLAG(type, name, value, doc)   type CONST_##name = value;
#else

Is it to find uses of developer/notproduct flags in product code?  Does it even matter?


On Oct 21, 2015, at 8:13 PM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>> wrote:

Hi Vladimir,

thanks for pushing the change!

Best regards,
 Goetz.


-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
Sent: Mittwoch, 21. Oktober 2015 17:10
To: Lindenmaier, Goetz; hotspot compiler
Subject: Re: RFR(XS): 8140239: Fix product build after "8132168: Support
IdealGraphVisualizer in optimized build"

Sure, looks good.

Best regards,
Vladimir Ivanov

On 10/21/15 5:34 PM, Lindenmaier, Goetz wrote:

Hi Vladimir,

I just tried to implement the same behavior as with 'develop'.
But excluding the flag altogether is fine with me, too:
http://cr.openjdk.java.net/~goetz/webrevs/8140239-prodBld/webrev.01/
I put in you as reviewer, if that's ok.

Best regards,
  Goetz.





-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
Sent: Mittwoch, 21. Oktober 2015 15:45
To: Lindenmaier, Goetz; hotspot compiler
Subject: Re: RFR(XS): 8140239: Fix product build after "8132168: Support
IdealGraphVisualizer in optimized build"

Goetz, thanks for spotting that!

Why not simply exclude IGVPrintLevel in product binaries instead?

-    cflags(IGVPrintLevel,           intx, PrintIdealGraphLevel,
IGVPrintLevel) \
+    NOT_PRODUCT(cflags(IGVPrintLevel, intx, PrintIdealGraphLevel,
IGVPrintLevel)) \

IGVPrintLevel is used only in non-product code and PrintIdealGraphLevel
functionality is not available in product builds.

Best regards,
Vladimir Ivanov

PS: I'm surprised the problem wasn't caught by JPRT. Shouldn't the
merged version be tested? It looks like the merge [1] happened after the
job finished. (FTR I had to restart the job due to a timeout.)

[1] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/03fa0a35a468

On 10/21/15 3:50 PM, Lindenmaier, Goetz wrote:

Hi,

After "8132168: Support IdealGraphVisualizer in optimized build" the
product build is broken:
compilerDirectives.cpp:181: error: 'PrintIdealGraphLevel' was not
declared in this scope

8132168 changes the flag from 'develop' to 'notproduct'. 'Notproduct'
does not define
"const intx PrintIdealGraphLevel = 0;" in the product build, as
'develop' did.

I fixed this by wrapping the flag with NOT_PRODUCT:

http://cr.openjdk.java.net/~goetz/webrevs/8140239-
prodBld/webrev.00/


Please review this change.  I please need a sponsor.

Best regards,

   Goetz.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151023/f09e3344/attachment.html>


More information about the hotspot-compiler-dev mailing list