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

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Feb 8 17:36:28 PST 2013


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


More information about the hotspot-dev mailing list