RFR: 8309136: [JVMCI] add -XX:+UseGraalJIT flag [v2]

Vladimir Kozlov kvn at openjdk.org
Wed May 31 22:18:08 UTC 2023


On Wed, 31 May 2023 08:08:34 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> Use of the Graal-based JIT in OpenJDK currently requires the following flag: `-XX:+EnableJVMCIProduct`
>> 
>> This has no direct association with Graal. If the JDK image happens to include a non-Graal JVMCI implementation, it will be automatically selected. This would come as a surprise to users who equate JVMCI with Graal.
>> 
>> This PR introduces a new flag, `-XX:+UseGraalJIT` to address these shortcomings. It is an alias for `-XX:+EnableJVMCIProduct -Djvmci.Compiler=graal`. 
>> 
>> When `-XX:+UseGraalJIT` is specified, the VM fails fast at startup if there is a non-Graal JVMCI implementation or no JVMCI implementation in the JDK image.
>
> Doug Simon has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review based fixes

Update Copyright year in test.

src/hotspot/share/runtime/arguments.cpp line 2834:

> 2832:         jio_fprintf(defaultStream::error_stream(),
> 2833:                   "-XX:-EnableJVMCIProduct or -XX:-UseGraalJIT cannot come after -XX:+EnableJVMCIProduct or -XX:+UseGraalJIT\n");
> 2834:         return JNI_EINVAL;

Why this restriction? Usually latest specified flag wins. May be comment with explanation.
Also you did not check for `if (UseGraalJIT)`. 
What about the case `-XX:+EnableJVMCIProduct -XX:-UseGraalJIT`?
I would expect this kind of checks done in `JVMCIGlobals::check_jvmci_flags_are_consistent()`

src/hotspot/share/runtime/arguments.cpp line 2840:

> 2838:         const char* jvmci_compiler = get_property("jvmci.Compiler");
> 2839:         if (jvmci_compiler != nullptr) {
> 2840:           if (strcmp(jvmci_compiler, "graal") != 0) {

You should not use `strcmp` - use `strncmp`.

test/hotspot/jtreg/compiler/jvmci/TestEnableJVMCIProduct.java line 78:

> 76:             new Expectation("UseJVMCICompiler", "false", "default"));
> 77:         test("-XX:+EnableJVMCIProduct",
> 78:             new Expectation("EnableJVMCIProduct", "true", "(?:command line|jimage)"),

What "(?:command line|jimage)" means? Also `|` hard to see.

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

PR Review: https://git.openjdk.org/jdk/pull/14231#pullrequestreview-1454237098
PR Review Comment: https://git.openjdk.org/jdk/pull/14231#discussion_r1212367355
PR Review Comment: https://git.openjdk.org/jdk/pull/14231#discussion_r1212359663
PR Review Comment: https://git.openjdk.org/jdk/pull/14231#discussion_r1212368628


More information about the hotspot-dev mailing list