Skip to content

8345826: Do not automatically resolve jdk.internal.vm.ci when libgraal is used #25240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

8345826: Do not automatically resolve jdk.internal.vm.ci when libgraal is used #25240

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
171703a
--add-modules=jdk.internal.vm.ci implies -XX:+EnableJVMCI
dougxc May 13, 2025
0e8773e
added comment in check_vm_args_consistency
dougxc May 15, 2025
3436033
resolve jdk.internal.vm.ci if +EnableJVMCI and -UseJVMCINativeLibrary
dougxc May 16, 2025
cc49ed0
fix TestHotSpotJVMCIRuntime
dougxc May 16, 2025
7b30238
removed use of jdk.internal.vm.ci.enabled property
dougxc May 16, 2025
3cdef58
fixed comment
dougxc May 16, 2025
1fe56b4
improved error message
dougxc May 16, 2025
cd615ef
JAVA_NOT_ENABLED_ERROR_MESSAGE -> JVMCI_NOT_ENABLED_ERROR_MESSAGE
dougxc May 16, 2025
d9223af
load the JVMCI module if +EnableJVMCI is set on the command line
dougxc May 16, 2025
196425f
load the JVMCI module if +EnableJVMCI is set in the jimage
dougxc May 18, 2025
b74077f
swapped order of recommended options in error message
dougxc May 19, 2025
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/jvmci/jvmciCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ JVMCICompiler::JVMCICompiler() : AbstractCompiler(compiler_jvmci) {

JVMCICompiler* JVMCICompiler::instance(bool require_non_null, TRAPS) {
if (!EnableJVMCI) {
THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), JAVA_NOT_ENABLED_ERROR_MESSAGE)
THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), JVMCI_NOT_ENABLED_ERROR_MESSAGE)
}
if (_instance == nullptr && require_non_null) {
THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), "The JVMCI compiler instance has not been created");
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/jvmci/jvmciRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ JRT_END
JVM_ENTRY_NO_ENV(jobject, JVM_GetJVMCIRuntime(JNIEnv *libjvmciOrHotspotEnv, jclass c))
JVMCIENV_FROM_JNI(thread, libjvmciOrHotspotEnv);
if (!EnableJVMCI) {
JVMCI_THROW_MSG_NULL(InternalError, JAVA_NOT_ENABLED_ERROR_MESSAGE);
JVMCI_THROW_MSG_NULL(InternalError, JVMCI_NOT_ENABLED_ERROR_MESSAGE);
}
JVMCIENV->runtime()->initialize_HotSpotJVMCIRuntime(JVMCI_CHECK_NULL);
JVMCIObject runtime = JVMCIENV->runtime()->get_HotSpotJVMCIRuntime(JVMCI_CHECK_NULL);
Expand All @@ -732,7 +732,7 @@ JVM_END
JVM_ENTRY_NO_ENV(jlong, JVM_ReadSystemPropertiesInfo(JNIEnv *env, jclass c, jintArray offsets_handle))
JVMCIENV_FROM_JNI(thread, env);
if (!EnableJVMCI) {
JVMCI_THROW_MSG_0(InternalError, JAVA_NOT_ENABLED_ERROR_MESSAGE);
JVMCI_THROW_MSG_0(InternalError, JVMCI_NOT_ENABLED_ERROR_MESSAGE);
}
JVMCIPrimitiveArray offsets = JVMCIENV->wrap(offsets_handle);
JVMCIENV->put_int_at(offsets, 0, SystemProperty::next_offset_in_bytes());
Expand Down Expand Up @@ -1515,7 +1515,7 @@ JVM_ENTRY_NO_ENV(void, JVM_RegisterJVMCINatives(JNIEnv *libjvmciOrHotspotEnv, jc
JVMCIENV_FROM_JNI(thread, libjvmciOrHotspotEnv);

if (!EnableJVMCI) {
JVMCI_THROW_MSG(InternalError, JAVA_NOT_ENABLED_ERROR_MESSAGE);
JVMCI_THROW_MSG(InternalError, JVMCI_NOT_ENABLED_ERROR_MESSAGE);
}

JVMCIENV->runtime()->initialize(JVMCIENV);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/jvmci/jvmciRuntime.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#include "gc/g1/g1CardTable.hpp"
#endif // INCLUDE_G1GC

#define JAVA_NOT_ENABLED_ERROR_MESSAGE "JVMCI is not enabled. Must specify '--add-modules=jdk.internal.vm.ci' or '-XX:+EnableJVMCI' to the java launcher."
#define JVMCI_NOT_ENABLED_ERROR_MESSAGE "JVMCI is not enabled. Must specify '--add-modules=jdk.internal.vm.ci' or '-XX:+EnableJVMCI' to the java launcher."
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the exception message for an InternalError so maybe this is okay but in general I don't think you want to be asking users to have to name a JDK internal module.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think anyone using a non-standard configuration for Graal would not be surprised about JVMCI and would actually appreciate the extra hint when they get things "wrong".

Copy link

Choose a reason for hiding this comment

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

Assuming we want users to use -XX:+EnableJVMCI over --add-modules=jdk.internal.vm.ci for said reason, maybe flip the order?

Copy link
Member Author

@dougxc dougxc May 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Done.


class JVMCIEnv;
class JVMCICompiler;
Expand Down
7 changes: 3 additions & 4 deletions src/hotspot/share/jvmci/jvmci_globals.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ class fileStream;
"Enable JVMCI support in the VM. " \
"Defaults to true if UseJVMCICompiler is true or " \
"--add-modules=jdk.internal.vm.ci was specified. " \
"If true and UseJVMCINativeLibrary is false (i.e. libjvmci is " \
"not available or not selected), the behavior of adding the " \
"jdk.internal.vm.ci module is triggered, as if the " \
"--add-modules=jdk.internal.vm.ci option was specified.") \
"If explicity set to true on the command line OR defaulted to " \
"true by UseJVMCICompiler and libjvmci is not enabled, the " \
"behavior --add-modules=jdk.internal.vm.ci is triggered.") \
\
product(bool, UseGraalJIT, false, EXPERIMENTAL, \
"Select the Graal JVMCI compiler. This is an alias for: " \
Expand Down
8 changes: 3 additions & 5 deletions src/hotspot/share/runtime/arguments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1801,11 +1801,9 @@ bool Arguments::check_vm_args_consistency() {
status = CompilerConfig::check_args_consistency(status);
#if INCLUDE_JVMCI
if (status && EnableJVMCI) {
// libjvmci doesn't require resolving jdk.internal.vm.ci as it is
// compiled into the libjvmci image itself. Without libjvmci, there
// is no other representation of the jdk.internal.vm.ci module
// so it needs to be added to the root module set.
if (!UseJVMCINativeLibrary && ClassLoader::is_module_observable("jdk.internal.vm.ci") && !_jvmci_module_added) {
// Add the JVMCI module if not using libjvmci or EnableJVMCI
// was explicitly set on the command line.
if ((!UseJVMCINativeLibrary || FLAG_IS_CMDLINE(EnableJVMCI)) && ClassLoader::is_module_observable("jdk.internal.vm.ci") && !_jvmci_module_added) {
if (!create_numbered_module_property("jdk.module.addmods", "jdk.internal.vm.ci", _addmods_count++)) {
return false;
}
Expand Down
1 change: 0 additions & 1 deletion ...iler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestHotSpotJVMCIRuntime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ public void jniEnomemTest() throws Exception {
ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(
"-XX:+UnlockExperimentalVMOptions",
"-XX:+EnableJVMCI",
"--add-modules=jdk.internal.vm.ci",
"-XX:-UseJVMCICompiler",
"-XX:+UseJVMCINativeLibrary",
"-Dtest.jvmci.forceEnomemOnLibjvmciInit=true",
Expand Down