RFR: 8257020: [JVMCI] enable a JVMCICompiler to specify which GCs it supports

[Doug Simon]

On Wed, 25 Nov 2020 13:34:24 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>>> Could this be done without adding JVMCI code to GCConfig::is_gc_supported?
>> 
>> This has to go through the WhiteBox API as JVMCI is not accessible to normal Java code (such as VMProps).
>> We could restrict the JVMCI interposition to WhiteBox. This is what I did [initially](https://github.com/openjdk/jdk/pull/1423/commits/191d61d996dd08f43c2c946f02b0cce0d96f2ef4). However, I then saw that `GCConfig::is_gc_supported` is only called from whitebox.cpp so [moved](https://github.com/openjdk/jdk/pull/1423/commits/54b7ba8463dfee9a679abb09a5cc898ba8550d85) the logic directly into `GCConfig`. I think this is better because as soon as there's another caller of `GCConfig::is_gc_supported`, they should get an accurate answer and not have to worry about implications of `EnableJVMCI` separately.
>
>> > Could this be done without adding JVMCI code to GCConfig::is_gc_supported?
>> 
>> This has to go through the WhiteBox API as JVMCI is not accessible to normal Java code (such as VMProps).
>> We could restrict the JVMCI interposition to WhiteBox. This is what I did [initially](https://github.com/openjdk/jdk/pull/1423/commits/191d61d996dd08f43c2c946f02b0cce0d96f2ef4). However, I then saw that `GCConfig::is_gc_supported` is only called from whitebox.cpp so [moved](https://github.com/openjdk/jdk/pull/1423/commits/54b7ba8463dfee9a679abb09a5cc898ba8550d85) the logic directly into `GCConfig`. I think this is better because as soon as there's another caller of `GCConfig::is_gc_supported`, they should get an accurate answer and not have to worry about implications of `EnableJVMCI` separately.
> 
> We already have code in place to ensure that EnableJVMCI is set to false when you run a GC that doesn't support JVMCI:
> void JVMCIGlobals::check_jvmci_supported_gc() {
>   if (EnableJVMCI) {
>     // Check if selected GC is supported by JVMCI and Java compiler
>     if (!(UseSerialGC || UseParallelGC || UseG1GC)) {
>       vm_exit_during_initialization("JVMCI Compiler does not support selected GC", GCConfig::hs_err_name());
>       FLAG_SET_DEFAULT(EnableJVMCI, false);
>       FLAG_SET_DEFAULT(UseJVMCICompiler, false);
>     }
>   }
> }
> 
> So, in our view we have "if you run this particular GC, that don't have support in JVMCI, then we turn off JVMCI forcefully", and we rely on that behavior in the HotSpot code. The proposed patch turns this inside out and the changed is_gc_supported function now becomes "if you've turned on EnableJVMCI is this GC supported". It sort-of inverts the dependency between the two sub-systems and we don't want to use that particular question in HotSpot. AFAICT, this is something you only need to ask in the context of jtreg's @requires, so I think it should be moved out of the GC code and put in the jtreg extension code.
> 
> We've other flags that we treat similarly. For example, UseCompressedOops. ZGC doesn't support it, so we turn it off forcefully, but we don't have code in GCConfig::is_gc_supported that returns false when UseCompressedOops is turned on. That is completely handled by the jtreg extensions (and the WhiteboxAPI).
> 
> I think you could easily solve this without adding calls through JVMCI from the GC code by:
> 1) Update JVMCIGlobals::supported_gc() to make a call through JVMCI into the used compiler that can tell if it supports the particular GC
> 2) Use Vladimirs proposed check_jvmci_supported_gc function, which uses supported_gc().
> 3) Add your own  WhiteboxAPI function, say WB_IsJVMCISupportedByGC, and call your "supported_gc" function from that
> 4) Update VMProps.java like I suggested above

`JVMCIGlobals::check_jvmci_supported_gc` is incorrect in that it hard codes the GCs supported by JVMCI which not something JVMCI itself can accurately answer. What's more, the upcall into JVMCI Java code to get the right answer cannot be made here as it is too early in the VM boot process. There's not even a `JavaThread` available yet. This code should be removed and let Graal exit the VM with an error should it be used in conjunction with a GC it does not support. For example:
Error occurred during initialization of VM
JVMCI Compiler does not support selected GC: epsilon gc

Moving the check to later in the VM bootstrap doesn't really work either as it imposes the overhead of initializing JVMCI and Graal eagerly.
This means a conflict between GC and JVMCI compiler cannot be finessed by overriding the compiler selection to use a non-JVMCI compiler. It's a fatal VM error, just like selecting multiple GCs on the command line is.

If you agree with that, then I think the proposed PR is the right approach. We could revert to the version that leaves the JVMCI specific logic in `WB_IsGCSupported` but as I stated, that means `GCConfig::is_gc_supported` can give a wrong answer.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1423