Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/compiler/compilerDefinitions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ bool CompilerConfig::check_args_consistency(bool status) {
FLAG_SET_DEFAULT(SegmentedCodeCache, false);
}
#if INCLUDE_JVMCI
if (EnableJVMCI) {
if (EnableJVMCI || UseJVMCICompiler) {
if (!FLAG_IS_DEFAULT(EnableJVMCI) || !FLAG_IS_DEFAULT(UseJVMCICompiler)) {
warning("JVMCI Compiler disabled due to -Xint.");
}
Expand Down
9 changes: 8 additions & 1 deletion src/hotspot/share/jvmci/jvmci_globals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ bool JVMCIGlobals::check_jvmci_flags_are_consistent() {
JVMCI_FLAG_CHECKED(UseJVMCICompiler)
JVMCI_FLAG_CHECKED(EnableJVMCI)
JVMCI_FLAG_CHECKED(EnableJVMCIProduct)
JVMCI_FLAG_CHECKED(UseGraalJIT)

CHECK_NOT_SET(BootstrapJVMCI, UseJVMCICompiler)
CHECK_NOT_SET(PrintBootstrap, UseJVMCICompiler)
Expand Down Expand Up @@ -164,7 +165,7 @@ bool JVMCIGlobals::check_jvmci_flags_are_consistent() {
}

// Convert JVMCI flags from experimental to product
bool JVMCIGlobals::enable_jvmci_product_mode(JVMFlagOrigin origin) {
bool JVMCIGlobals::enable_jvmci_product_mode(JVMFlagOrigin origin, bool use_graal_jit) {
const char *JVMCIFlags[] = {
"EnableJVMCI",
"EnableJVMCIProduct",
Expand Down Expand Up @@ -201,6 +202,12 @@ bool JVMCIGlobals::enable_jvmci_product_mode(JVMFlagOrigin origin) {
if (JVMFlagAccess::set_bool(jvmciEnableFlag, &value, origin) != JVMFlag::SUCCESS) {
return false;
}
if (use_graal_jit) {
JVMFlag *useGraalJITFlag = JVMFlag::find_flag("UseGraalJIT");
if (JVMFlagAccess::set_bool(useGraalJITFlag, &value, origin) != JVMFlag::SUCCESS) {
return false;
}
}

// Effect of EnableJVMCIProduct on changing defaults of EnableJVMCI
// and UseJVMCICompiler is deferred to check_jvmci_flags_are_consistent
Expand Down
7 changes: 6 additions & 1 deletion src/hotspot/share/jvmci/jvmci_globals.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ class fileStream;
product(bool, EnableJVMCI, false, EXPERIMENTAL, \
"Enable JVMCI") \
\
product(bool, UseGraalJIT, false, EXPERIMENTAL, \
"Select the Graal JVMCI compiler. This is an alias for: " \
" -XX:+EnableJVMCIProduct " \
" -Djvmci.Compiler=graal ") \
\
product(bool, EnableJVMCIProduct, false, EXPERIMENTAL, \
"Allow JVMCI to be used in product mode. This alters a subset of "\
"JVMCI flags to be non-experimental, defaults UseJVMCICompiler " \
Expand Down Expand Up @@ -185,7 +190,7 @@ class JVMCIGlobals {
static bool check_jvmci_flags_are_consistent();

// Convert JVMCI experimental flags to product
static bool enable_jvmci_product_mode(JVMFlagOrigin);
static bool enable_jvmci_product_mode(JVMFlagOrigin origin, bool use_graal_jit);

// Returns true iff the GC fully supports JVMCI.
static bool gc_supports_jvmci();
Expand Down
26 changes: 20 additions & 6 deletions src/hotspot/share/runtime/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2827,28 +2827,42 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
return JNI_ERR;
#endif // INCLUDE_MANAGEMENT
#if INCLUDE_JVMCI
} else if (match_option(option, "-XX:-EnableJVMCIProduct")) {
} else if (match_option(option, "-XX:-EnableJVMCIProduct") || match_option(option, "-XX:-UseGraalJIT")) {
if (EnableJVMCIProduct) {
jio_fprintf(defaultStream::error_stream(),
"-XX:-EnableJVMCIProduct cannot come after -XX:+EnableJVMCIProduct\n");
"-XX:-EnableJVMCIProduct or -XX:-UseGraalJIT cannot come after -XX:+EnableJVMCIProduct or -XX:+UseGraalJIT\n");
return JNI_EINVAL;
Comment on lines 2832 to 2834
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this restriction? Usually latest specified flag wins. May be comment with explanation.

This is a pre-existing restriction on EnableJVMCIProduct and since UseGraalJIT is an alias for EnableJVMCIProduct, it must abide by the same restrictions. For whatever reason, the effect of +EnableJVMCIProduct cannot be undone and so the same must hold for +UseGraalJIT.

Also you did not check for if (UseGraalJIT).

This check is not necessary as UseGraalJIT is an alias. When EnableJVMCIProduct is set to true, UseGraalJIT will also be set to true. Stating this now, I see that I missed it: 430b7a5

What about the case -XX:+EnableJVMCIProduct -XX:-UseGraalJIT?

That results in:

-XX:-EnableJVMCIProduct or -XX:-UseGraalJIT cannot come after -XX:+EnableJVMCIProduct or -XX:+UseGraalJIT
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

which is what is expected.

}
} else if (match_option(option, "-XX:+EnableJVMCIProduct")) {
// Just continue, since "-XX:+EnableJVMCIProduct" has been specified before
} else if (match_option(option, "-XX:+EnableJVMCIProduct") || match_option(option, "-XX:+UseGraalJIT")) {
bool use_graal_jit = match_option(option, "-XX:+UseGraalJIT");
if (use_graal_jit) {
const char* jvmci_compiler = get_property("jvmci.Compiler");
if (jvmci_compiler != nullptr) {
if (strncmp(jvmci_compiler, "graal", strlen("graal")) != 0) {
jio_fprintf(defaultStream::error_stream(),
"Value of jvmci.Compiler incompatible with +UseGraalJIT: %s", jvmci_compiler);
return JNI_ERR;
}
} else if (!add_property("jvmci.Compiler=graal")) {
return JNI_ENOMEM;
}
}

// Just continue, since "-XX:+EnableJVMCIProduct" or "-XX:+UseGraalJIT" has been specified before
if (EnableJVMCIProduct) {
continue;
}
JVMFlag *jvmciFlag = JVMFlag::find_flag("EnableJVMCIProduct");
// Allow this flag if it has been unlocked.
if (jvmciFlag != nullptr && jvmciFlag->is_unlocked()) {
if (!JVMCIGlobals::enable_jvmci_product_mode(origin)) {
if (!JVMCIGlobals::enable_jvmci_product_mode(origin, use_graal_jit)) {
jio_fprintf(defaultStream::error_stream(),
"Unable to enable JVMCI in product mode");
return JNI_ERR;
}
}
// The flag was locked so process normally to report that error
else if (!process_argument("EnableJVMCIProduct", args->ignoreUnrecognized, origin)) {
else if (!process_argument(use_graal_jit ? "UseGraalJIT" : "EnableJVMCIProduct", args->ignoreUnrecognized, origin)) {
return JNI_EINVAL;
}
#endif // INCLUDE_JVMCI
Expand Down
45 changes: 31 additions & 14 deletions test/hotspot/jtreg/compiler/jvmci/TestEnableJVMCIProduct.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -50,6 +50,14 @@ static class Expectation {
}

public static void main(String[] args) throws Exception {
if (args.length != 0) {
// Called as subprocess. Print system properties named by
// `args` and then exit.
for (String arg : args) {
System.out.printf("%s=%s%n", arg, System.getProperty(arg));
}
return;
}
// Test EnableJVMCIProduct without any other explicit JVMCI option
test("-XX:-PrintWarnings",
new Expectation("EnableJVMCI", "true", "default"),
Expand All @@ -67,24 +75,33 @@ public static void main(String[] args) throws Exception {
new Expectation("EnableJVMCI", "false", "command line"),
new Expectation("UseJVMCICompiler", "false", "default"));
test("-XX:+EnableJVMCIProduct",
new Expectation("EnableJVMCIProduct", "true", "command line"),
new Expectation("EnableJVMCIProduct", "true", "(?:command line|jimage)"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a standard Java regular expression and it means the value is expected have been set on the command line or by the VM options read from the jimage (see JDK-8232080).

new Expectation("EnableJVMCI", "true", "default"),
new Expectation("UseJVMCICompiler", "true", "default"));
}

static void test(String explicitFlag, Expectation... expectations) throws Exception {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
"-XX:+UnlockExperimentalVMOptions", "-XX:+EnableJVMCIProduct", "-XX:-UnlockExperimentalVMOptions",
explicitFlag,
"-XX:+PrintFlagsFinal", "-version");
OutputAnalyzer output = new OutputAnalyzer(pb.start());
for (Expectation expectation : expectations) {
output.stdoutShouldMatch(expectation.pattern);
}
if (output.getExitValue() != 0) {
// This should only happen when JVMCI compilation is requested and the VM has no
// JVMCI compiler (e.g. Graal is not included in the build)
output.stdoutShouldMatch("No JVMCI compiler found");
String[] flags = {"-XX:+EnableJVMCIProduct", "-XX:+UseGraalJIT"};
String cwd = System.getProperty("user.dir");
for (String flag : flags) {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
"-XX:+UnlockExperimentalVMOptions", flag, "-XX:-UnlockExperimentalVMOptions",
explicitFlag,
"-XX:+PrintFlagsFinal",
"--class-path=" + System.getProperty("java.class.path"),
"TestEnableJVMCIProduct", "jvmci.Compiler");
OutputAnalyzer output = new OutputAnalyzer(pb.start());
for (Expectation expectation : expectations) {
output.stdoutShouldMatch(expectation.pattern);
}
if (flag.equals("-XX:+UseGraalJIT")) {
output.shouldContain("jvmci.Compiler=graal");
}
if (output.getExitValue() != 0) {
// This should only happen when JVMCI compilation is requested and the VM has no
// JVMCI compiler (e.g. Graal is not included in the build)
output.stdoutShouldMatch("No JVMCI compiler found");
}
}
}
}