[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Re: RFR(XS) : 8021120 : TieredCompilation can be enabled even if TIERED is undefined
From:       David Holmes <david.holmes () oracle ! com>
Date:       2013-07-29 1:15:27
Message-ID: 51F5C22F.6000200 () oracle ! com
[Download RAW message or body]

Thanks - looks good.

David

On 28/07/2013 7:28 PM, Igor Ignatyev wrote:
> David,
>
> I updated webrev according to your suggestion: used UNSUPPORTED_OPTION
> as-is.
>
> webrev: http://cr.openjdk.java.net/~iignatyev/8021120/webrev.02/
> testing: the same as before: jprt, 'java -version' on different VM
>
> Best regards,
> Igor Ignatyev
>
> On 07/25/2013 09:33 AM, Vladimir Kozlov wrote:
>> On 7/24/13 10:16 PM, David Holmes wrote:
>>> On 25/07/2013 12:31 AM, Vladimir Kozlov wrote:
>>>> On 7/24/13 4:29 AM, David Holmes wrote:
>>>>> On 24/07/2013 9:12 AM, Vladimir Kozlov wrote:
>>>>>> David,
>>>>>>
>>>>>> We can't use UNSUPPORTED_OPTION because of the message it produces:
>>>>>> "is
>>>>>> disabled in this release." We need: "is not supported in this VM."
>>>>>
>>>>> Do the exact words really matter? If it is disabled it is not
>>>>> supported.
>>>>>
>>>>> But the patch issues no warning it simply ignores what the user has
>>>>> asked for. I prefer it when people get told they are
>>>>> asking for something they can't have.
>>>>
>>>> Last version Igor sent has warning:
>>>>
>>>> http://cr.openjdk.java.net/~iignatyev/8021120/webrev.01/
>>>
>>> I never saw that one as it only went to hotspot-compiler-dev. (The
>>> other alias should not be cc'd and I've dropped it -
>>> which means I won't see any replies unless explicitly cc'd :) ).
>>>
>>> I still don't see that the UNSUPPORTED_OPTION wording should be an
>>> issue here. If you really think it is then rework the
>>> macro to allow the main text to be passed as well. But this seems a
>>> non-issue to me. After all who are we targetting
>>> with this?
>>
>> Embedded VM :) Embedded Server VM does not support Tiered (is not built
>> with it).
>>
>> Anyway, I looked and currently UNSUPPORTED_OPTION is used only in 2
>> places (I thought a lot more). And in both places it really mean "not
>> supported":
>>
>> #ifdef _ALLBSD_SOURCE  // UseLargePages is not yet supported on BSD.
>>    UNSUPPORTED_OPTION(UseLargePages, "-XX:+UseLargePages");
>> #endif
>>
>> #if INCLUDE_ALL_GCS
>>    #if (defined JAVASE_EMBEDDED || defined ARM)
>>      UNSUPPORTED_OPTION(UseG1GC, "G1 GC");
>>    #endif
>> #endif
>>
>> So I agree with you now that we can change message in
>> UNSUPPORTED_OPTION to
>>
>> warning(description " is not supported in this VM.");
>>
>> Igor, please, do as David suggested, modify message and use
>> UNSUPPORTED_OPTION.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> David
>>> -----
>>>
>>>> Vladimir
>>>>
>>>>>
>>>>> David
>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 7/23/13 4:04 PM, David Holmes wrote:
>>>>>>> Hi Igor,
>>>>>>>
>>>>>>> On 23/07/2013 10:38 PM, Igor Ignatyev wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> Please review patch.
>>>>>>>>
>>>>>>>> Problem:
>>>>>>>> If 'COMPILER2' is defined, it will be possible to enable
>>>>>>>> 'TieredCompilation' even if 'TIERED' is undefined. But such VM will
>>>>>>>> raise error at 'compilationPolicy_init'
>>>>>>>>
>>>>>>>> Fix:
>>>>>>>> Forcibly set 'TieredCompilation' in false, if 'TIERED' is
>>>>>>>> undefined.
>>>>>>>
>>>>>>> If you use UNSUPPORTED_OPTION it will issue a warning and force
>>>>>>> it to
>>>>>>> false.
>>>>>>>
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8021120/webrev.00/
>>>>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8021120
>>>>>>>> testing: jprt, 'java -version' on different VM (including embedded
>>>>>>>> client, server, minimal)
[prev in list] [next in list] [prev in thread] [next in thread]