Request for review (XS): JDK-8006430: TraceTypeProfile is a product flag while it should be a diagnostic flag

[Krystal Mo]

Hi Vladimir,

Thank you for the review. I didn't add any C1 related checks because C1 
doesn't have diagnostic/experimental flags declared. If I were to add 
those kinds, I'd touch more files then I'd like.
Perhaps a new RFE for adding C1 diagnostic/experimental flag kinds?

P.S. Oops, I forgot to update the copyright year. Will be fixed before I 
push.

Thanks,
Kris

On 02/08/2013 05:36 PM, Vladimir Kozlov wrote:
> Looks good. I think you can preemptively add "{C1 diagnostic}".
>
> Thanks,
> Vladimir
>
> On 2/8/13 5:23 PM, Krystal Mo wrote:
>> Hi all,
>>
>> Could I have a couple of reviews for this fix, please?
>>
>> Webrev: http://cr.openjdk.java.net/~kmo/8006430/webrev.00/
>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8006430
>>
>> This bug is actually not a regression from JDK-7198499, but a long time
>> issue that C2 diagnostic flags were never treated as being "diagnostic"
>> since the OpenJDK Mercurial repo history began (duke at 0).
>>
>> The problem is, C2 diagnostic flags have the kind "{C2 diagnostic}", but
>> Flag::is_unlocked() only cared to test "{diagnostic}".
>> So the fix is straightforward -- find out how many other "diagnostic" 
>> and
>> "experimental" flag kinds there are, and check them.
>>
>> I didn't just check if the string ends with "diagnostic}" or
>> "experimental}" because I think that's too permissive, but I don't have
>> a strong opinion on that.
>> If reviews form a consensus that checking just the postfix is good
>> enough, I can make the change that way.
>>
>> Care has to be taken if new diagnostic/experimental flag kinds are added
>> to other components in the future, e.g. C1.
>>
>> Tested with JPRT and these flags by hand:
>>    C2 diagnostic: TraceTypeProfile, LoopLimitCheck, RangeLimitCheck
>>    ARCH diagnostic: UseIncDec
>>
>> Thanks,
>> Kris