RFR(XS): 8140239: Fix product build after "8132168: Support IdealGraphVisualizer in optimized build"
Christian Thalinger
christian.thalinger at oracle.com
Fri Oct 23 20:36:00 UTC 2015
> On Oct 23, 2015, at 9:20 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
>
> Hi,
>
> Yes, I think that is better, too.
> I just wanted to make the quick fix for the build, first.
Sure. Let me file an enhancement for what I proposed…
https://bugs.openjdk.java.net/browse/JDK-8140424
>
> 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 <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 <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/ <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 <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 <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- <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/79937a02/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list