RFR: 8161265: [JVMCI] EnableJVMCI should only be required when its not implied by other flags

[Christian Thalinger]

> On Jul 14, 2016, at 1:12 PM, Doug Simon <doug.simon at oracle.com> wrote:
> 
>> 
>> On 15 Jul 2016, at 00:46, Christian Thalinger <cthalinger at twitter.com> wrote:
>> 
>>> 
>>> On Jul 14, 2016, at 12:03 PM, Doug Simon <doug.simon at oracle.com> wrote:
>>> 
>>>> 
>>>> On 14 Jul 2016, at 21:12, Tom Rodriguez <tom.rodriguez at oracle.com> wrote:
>>>> 
>>>>> 
>>>>> Compared to the logic that was there, it’s a huge step forward! There’s so few flags and they’re very unlikely to change that I think the trade off is worth it.
>>>>> 
>>>>> If there was some easy way to get the number of flags created by JVMCI_FLAGS, we could add an assert in JVMCIGlobals::check_jvmci_flags_are_consistent against an EXPECTED_JVMCI_FLAGS constant. That would be sufficient as removal of a flag will cause a compilation error, and addition of a flag (without adjusting EXPECTED_JVMCI_FLAGS) will cause a guarantee failure. The only problem is that I don’t see how to get the number of flags created by JVMCI_FLAGS.
>>>>> 
>>>>> -Doug
>>>> 
>>>> You can do anything with macros.  A macro using JVMCI_FLAGS could easily count them.
>>> 
>>> Yes, but then I have to define the secondary macros as parameters for application of JVMCI_FLAGS and I wanted to avoid that.
>>> 
>>>> Why not have a macro that generates local bools for each JVMCI flag that checks whether it has been sanity checked.  Your new macros would set the appropriate flags and and a final call to JVMCI_FLAGS would assert that they’ve all been sanity checked.
>>> 
>>> Ok, I’ve implemented that here:
>>> 
>>> http://cr.openjdk.java.net/~dnsimon/8161265.v2/index.html
>> 
>> This is super painful; even more for you than me :-)
> 
> The only thing that makes it painful is that openjdk is still on 20 year old code reviewing technology.

Oh yes.

> 
>> With your changes what happens if you do:
>> 
>> -XX:+UseJVMCICompiler -XX:-EnableJVMCI
> 
> At the moment, it ignores the latter which is probably going to surprise someone. This fixes it:
> 
> http://cr.openjdk.java.net/~dnsimon/8161265.v3/index.html <http://cr.openjdk.java.net/~dnsimon/8161265.v3/index.html>

As far as I can tell this looks good.  I hope existing tests have good coverage.

> 
> -Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160714/cf3061c2/attachment-0001.html>