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

Vladimir Kozlov vladimir.kozlov at oracle.com
Sat Oct 24 02:53:19 UTC 2015 

 > +#define NAME(name) (void*)&name

If it works with all C++ compilers I am fine with it.

Thanks,
Vladimir

On 10/24/15 2:39 AM, Christian Thalinger wrote:
>
>> 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.cppMon Oct 19 00:24:58 2015 -0700
> +++ b/src/share/vm/runtime/globals.cppFri 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.hppMon Oct 19 00:24:58 2015 -0700
> +++ b/src/share/vm/runtime/globals.hppFri 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.
>>>>>>>>
>>>
>

More information about the hotspot-compiler-dev mailing list