RFR: 8264806: Remove the experimental JIT compiler
As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to Java-based JIT compiler (Graal) from JDK: - `jdk.internal.vm.compiler` — the Graal compiler - `jdk.internal.vm.compiler.management` — Graal's `MBean` - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests Remove Graal related code in makefiles. Note, next two `module-info.java` files are preserved so that the JVMCI module `jdk.internal.vm.ci` continues to build: src/jdk.internal.vm.compiler/share/classes/module-info.java src/jdk.internal.vm.compiler.management/share/classes/module-info.java @AlanBateman suggested that we can avoid it by using Module API to export packages at runtime . It requires changes in GraalVM's JVMCI too so I will file followup RFE to implement it. Tested hs-tier1-4 - Depends on: https://git.openjdk.java.net/jdk/pull/3398 Commit messages: - 8264806: Remove the experimental JIT compiler Changes: https://git.openjdk.java.net/jdk/pull/3421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264806 Stats: 441620 lines in 2886 files changed: 0 ins; 441604 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler
On Fri, 9 Apr 2021 17:35:11 GMT, Vladimir Kozlov wrote: > As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 make/GraalBuilderImage.gmk line 1: > 1: # This file should not be removed. This outout image is this makefile produces is used as input to the separate GraalVM build. make/Main.gmk line 444: > 442: )) > 443: > 444: $(eval $(call SetupTarget, graal-builder-image, \ Same as above, this needs to stay. make/autoconf/spec.gmk.in line 895: > 893: STATIC_LIBS_IMAGE_DIR := $(IMAGES_OUTPUTDIR)/$(STATIC_LIBS_IMAGE_SUBDIR) > 894: > 895: # Graal builder image Like above, this needs to stay. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler
On Fri, 9 Apr 2021 17:35:11 GMT, Vladimir Kozlov wrote: > As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Thankyou, @erikj79, for review. I restored code as you asked. For some reasons incremental webrev shows update only in Cdiffs. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler
On Fri, 9 Apr 2021 22:30:32 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Java-based JIT compiler (Graal) from JDK: >> >> - `jdk.internal.vm.compiler` — the Graal compiler >> - `jdk.internal.vm.compiler.management` — Graal's `MBean` >> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests >> >> Remove Graal related code in makefiles. >> >> Note, next two `module-info.java` files are preserved so that the JVMCI >> module `jdk.internal.vm.ci` continues to build: >> src/jdk.internal.vm.compiler/share/classes/module-info.java >> src/jdk.internal.vm.compiler.management/share/classes/module-info.java >> >> @AlanBateman suggested that we can avoid it by using Module API to export >> packages at runtime . It requires changes in GraalVM's JVMCI too so I will >> file followup RFE to implement it. >> >> Tested hs-tier1-4 > > Thankyou, @erikj79, for review. I restored code as you asked. > For some reasons incremental webrev shows update only in Cdiffs. none of the full webrevs seem to render even the list of changed files? is it just me? - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Restore Graal Builder image makefile - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 - 8264806: Remove the experimental JIT compiler - Changes: - all: https://git.openjdk.java.net/jdk/pull/3421/files - new: https://git.openjdk.java.net/jdk/pull/3421/files/8ff9b599..a246aaa6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=00-01 Stats: 102 lines in 12 files changed: 66 ins; 27 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Java-based JIT compiler (Graal) from JDK: >> >> - `jdk.internal.vm.compiler` — the Graal compiler >> - `jdk.internal.vm.compiler.management` — Graal's `MBean` >> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests >> >> Remove Graal related code in makefiles. >> >> Note, next two `module-info.java` files are preserved so that the JVMCI >> module `jdk.internal.vm.ci` continues to build: >> src/jdk.internal.vm.compiler/share/classes/module-info.java >> src/jdk.internal.vm.compiler.management/share/classes/module-info.java >> >> @AlanBateman suggested that we can avoid it by using Module API to export >> packages at runtime . It requires changes in GraalVM's JVMCI too so I will >> file followup RFE to implement it. >> >> Tested hs-tier1-4 > > Vladimir Kozlov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Restore Graal Builder image makefile > - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 > - 8264806: Remove the experimental JIT compiler should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and update a few of its users accordingly? what about `vm.graal.enabled` `@requires` property? - Changes requested by iignatyev (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 15:38:11 GMT, Igor Ignatyev wrote: > should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and > update a few of its users accordingly? > what about `vm.graal.enabled` `@requires` property? Thank you, @iignatev for looking on changes. I forgot to mention that `Compiler::isGraalEnabled()` returns always false now. Because 94 tests uses `@requires !vm.graal.enabled` I don't want to include them in these changes which are already very big. I am not sure if I should modify tests if GraalVM group wants to run all these tests. Unfortunately changes in `Compiler.java` are listed the last on `Files changed` tab and GitHub has trouble to load these big changes - it takes time to see them. Here `Compiler.java` chnges for review: diff --git a/test/lib/sun/hotspot/code/Compiler.java b/test/lib/sun/hotspot/code/Compiler.java index 99122bd93b8..71288ae4482 100644 --- a/test/lib/sun/hotspot/code/Compiler.java +++ b/test/lib/sun/hotspot/code/Compiler.java @@ -60,33 +60,10 @@ public class Compiler { /** * Check if Graal is used as JIT compiler. * - * Graal is enabled if following conditions are true: - * - we are not in Interpreter mode - * - UseJVMCICompiler flag is true - * - jvmci.Compiler variable is equal to 'graal' - * - TieredCompilation is not used or TieredStopAtLevel is greater than 3 - * No need to check client mode because it set UseJVMCICompiler to false. - * - * @return true if Graal is used as JIT compiler. + * @return false because Graal is removed from JDK. */ public static boolean isGraalEnabled() { -Boolean useCompiler = WB.getBooleanVMFlag("UseCompiler"); -if (useCompiler == null || !useCompiler) { -return false; -} -Boolean useJvmciComp = WB.getBooleanVMFlag("UseJVMCICompiler"); -if (useJvmciComp == null || !useJvmciComp) { -return false; -} - -Boolean tieredCompilation = WB.getBooleanVMFlag("TieredCompilation"); -Long compLevel = WB.getIntxVMFlag("TieredStopAtLevel"); -// if TieredCompilation is enabled and compilation level is <= 3 then no Graal is used -if (tieredCompilation != null && tieredCompilation && -compLevel != null && compLevel <= 3) { -return false; -} -return true; +return false; } ``` - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 15:38:11 GMT, Igor Ignatyev wrote: >> Vladimir Kozlov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and > update a few of its users accordingly? > what about `vm.graal.enabled` `@requires` property? @iignatev If you think that I should clean tests anyway I will file follow up RFE to do that. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Java-based JIT compiler (Graal) from JDK: >> >> - `jdk.internal.vm.compiler` — the Graal compiler >> - `jdk.internal.vm.compiler.management` — Graal's `MBean` >> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests >> >> Remove Graal related code in makefiles. >> >> Note, next two `module-info.java` files are preserved so that the JVMCI >> module `jdk.internal.vm.ci` continues to build: >> src/jdk.internal.vm.compiler/share/classes/module-info.java >> src/jdk.internal.vm.compiler.management/share/classes/module-info.java >> >> @AlanBateman suggested that we can avoid it by using Module API to export >> packages at runtime . It requires changes in GraalVM's JVMCI too so I will >> file followup RFE to implement it. >> >> Tested hs-tier1-4 > > Vladimir Kozlov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Restore Graal Builder image makefile > - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 > - 8264806: Remove the experimental JIT compiler Marked as reviewed by iignatyev (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 16:36:54 GMT, Vladimir Kozlov wrote: >> should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and >> update a few of its users accordingly? >> what about `vm.graal.enabled` `@requires` property? > > @iignatev If you think that I should clean tests anyway I will file follow > up RFE to do that. > > should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and > > update a few of its users accordingly? > > what about `vm.graal.enabled` `@requires` property? > > Thank you, @iignatev for looking on changes. > > I forgot to mention that `Compiler::isGraalEnabled()` returns always false > now. Because 94 tests uses `@requires !vm.graal.enabled` I don't want to > include them in these changes which are already very big. I am not sure if I > should modify tests if GraalVM group wants to run all these tests. > If you think that I should clean tests anyway I will file follow up RFE to do > that. changing `Compiler::isGraalEnabled()` to always return false effectively makes these tests unrunnable for GraalVM group (unless they are keep the modified `sun/hotspot/code/Compiler` and/or `requires/VMProps` in their forks). on top of that, I foresee that there will be more tests incompatible w/ Graal yet given it won't be run/tested in OpenJDK, these tests won't be marked and hence will fail when run w/ Graal. so Graal people will have to either do marking themselves (I guess in both upstream and their fork) or maintain a list of inapplicable tests in a format that works best for their setup. that's to say, I think we should clean up the tests, yet I totally agree there is no need to do it as part of this PR. we can discuss how to do it better for both OpenJDK and GraalVM folks in the follow-up RFE. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 16:47:45 GMT, Igor Ignatyev wrote: >> Vladimir Kozlov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > Marked as reviewed by iignatyev (Reviewer). Thank you, Igor. I filed https://bugs.openjdk.java.net/browse/JDK-8265032 - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 17:41:05 GMT, Vladimir Kozlov wrote: >> Marked as reviewed by iignatyev (Reviewer). > > Thank you, Igor. I filed https://bugs.openjdk.java.net/browse/JDK-8265032 We would definitely like to be able to continue testing of GraalVM with the JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like it does today is important. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Java-based JIT compiler (Graal) from JDK: >> >> - `jdk.internal.vm.compiler` — the Graal compiler >> - `jdk.internal.vm.compiler.management` — Graal's `MBean` >> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests >> >> Remove Graal related code in makefiles. >> >> Note, next two `module-info.java` files are preserved so that the JVMCI >> module `jdk.internal.vm.ci` continues to build: >> >> src/jdk.internal.vm.compiler/share/classes/module-info.java >> src/jdk.internal.vm.compiler.management/share/classes/module-info.java >> >> >> @AlanBateman suggested that we can avoid it by using Module API to export >> packages at runtime . It requires changes in GraalVM's JVMCI too so I will >> file followup RFE to implement it. >> >> Tested hs-tier1-4 > > Vladimir Kozlov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Restore Graal Builder image makefile > - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 > - 8264806: Remove the experimental JIT compiler make/common/Modules.gmk line 68: > 66: > 67: # Filter out Graal specific modules > 68: MODULES_FILTER += jdk.internal.vm.compiler If we are unconditionally filtering out these modules, then why leave the module-info.java files in at all? - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Mon, 12 Apr 2021 16:18:32 GMT, Erik Joelsson wrote: >> Vladimir Kozlov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > make/common/Modules.gmk line 68: > >> 66: >> 67: # Filter out Graal specific modules >> 68: MODULES_FILTER += jdk.internal.vm.compiler > > If we are unconditionally filtering out these modules, then why leave the > module-info.java files in at all? We filter out because we can't build Graal anymore. But we need these module-info.java files because JVMCI's module-info.java references them: https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26 Otherwise we can't build JVMCI which we continue to support. I filed followup RFE to implement Alan's suggestion to use Module API which will allow to remove these files later: https://bugs.openjdk.java.net/browse/JDK-8265091 - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Mon, 12 Apr 2021 17:18:36 GMT, Vladimir Kozlov wrote: >> make/common/Modules.gmk line 68: >> >>> 66: >>> 67: # Filter out Graal specific modules >>> 68: MODULES_FILTER += jdk.internal.vm.compiler >> >> If we are unconditionally filtering out these modules, then why leave the >> module-info.java files in at all? > > We filter out because we can't build Graal anymore. But we need these > module-info.java files because JVMCI's module-info.java references them: > https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26 > > Otherwise we can't build JVMCI which we continue to support. > > I filed followup RFE to implement Alan's suggestion to use Module API which > will allow to remove these files later: > https://bugs.openjdk.java.net/browse/JDK-8265091 Right, I thought I saw something about modules that Alan commented on, but couldn't find it. All good then. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Java-based JIT compiler (Graal) from JDK: >> >> - `jdk.internal.vm.compiler` — the Graal compiler >> - `jdk.internal.vm.compiler.management` — Graal's `MBean` >> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests >> >> Remove Graal related code in makefiles. >> >> Note, next two `module-info.java` files are preserved so that the JVMCI >> module `jdk.internal.vm.ci` continues to build: >> >> src/jdk.internal.vm.compiler/share/classes/module-info.java >> src/jdk.internal.vm.compiler.management/share/classes/module-info.java >> >> >> @AlanBateman suggested that we can avoid it by using Module API to export >> packages at runtime . It requires changes in GraalVM's JVMCI too so I will >> file followup RFE to implement it. >> >> Tested hs-tier1-4 > > Vladimir Kozlov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Restore Graal Builder image makefile > - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 > - 8264806: Remove the experimental JIT compiler Build changes look good. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v3]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Restore Compiler::isGraalEnabled() - Changes: - all: https://git.openjdk.java.net/jdk/pull/3421/files - new: https://git.openjdk.java.net/jdk/pull/3421/files/a246aaa6..9d6bd42c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=01-02 Stats: 25 lines in 1 file changed: 23 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sun, 11 Apr 2021 10:25:47 GMT, Doug Simon wrote: >> Vladimir Kozlov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > We would definitely like to be able to continue testing of GraalVM with the > JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like > it does today is important. @dougxc I restored Compiler::isGraalEnabled(). - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sun, 11 Apr 2021 10:25:47 GMT, Doug Simon wrote: >> Vladimir Kozlov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > We would definitely like to be able to continue testing of GraalVM with the > JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like > it does today is important. > @dougxc I restored Compiler::isGraalEnabled(). Thanks. I guess this should really be named `isJVMCICompilerEnabled` now and the `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but maybe that's too big a change (or can be done later). - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Tue, 13 Apr 2021 09:30:23 GMT, Doug Simon wrote: >> We would definitely like to be able to continue testing of GraalVM with the >> JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like >> it does today is important. > >> @dougxc I restored Compiler::isGraalEnabled(). > > Thanks. I guess this should really be named `isJVMCICompilerEnabled` now and > the `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but > maybe that's too big a change (or can be done later). @dougxc Renaming should be done separately. May be use RFE I filed: https://bugs.openjdk.java.net/browse/JDK-8265032 Do you approve these Graal removal changes? - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v3]
On Mon, 12 Apr 2021 22:10:06 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Java-based JIT compiler (Graal) from JDK: >> >> - `jdk.internal.vm.compiler` — the Graal compiler >> - `jdk.internal.vm.compiler.management` — Graal's `MBean` >> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests >> >> Remove Graal related code in makefiles. >> >> Note, next two `module-info.java` files are preserved so that the JVMCI >> module `jdk.internal.vm.ci` continues to build: >> >> src/jdk.internal.vm.compiler/share/classes/module-info.java >> src/jdk.internal.vm.compiler.management/share/classes/module-info.java >> >> >> @AlanBateman suggested that we can avoid it by using Module API to export >> packages at runtime . It requires changes in GraalVM's JVMCI too so I will >> file followup RFE to implement it. >> >> Tested hs-tier1-4 > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Restore Compiler::isGraalEnabled() Approved. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v4]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Merge branch 'JDK-8264805' into JDK-8264806 - Merge branch 'master' into JDK-8264805 - Restore Compiler::isGraalEnabled() - Restore Graal Builder image makefile - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 - Address review comments - 8264806: Remove the experimental JIT compiler - Remove exports from Graal module to jdk.aot - Merge branch 'master' into JDK-8264805 - Merge branch 'master' into JDK-8264805 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/b524a81a...db7c9aaf - Changes: https://git.openjdk.java.net/jdk/pull/3421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=03 Stats: 468493 lines in 3247 files changed: 2 ins; 468290 del; 201 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v5]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Merge branch 'master' into JDK-8264806 - Merge branch 'JDK-8264805' into JDK-8264806 - Merge branch 'master' into JDK-8264805 - Restore Compiler::isGraalEnabled() - Restore Graal Builder image makefile - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 - Address review comments - 8264806: Remove the experimental JIT compiler - Remove exports from Graal module to jdk.aot - Merge branch 'master' into JDK-8264805 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/694acedf...b1e9ba63 - Changes: https://git.openjdk.java.net/jdk/pull/3421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3421&range=04 Stats: 441532 lines in 2884 files changed: 0 ins; 441518 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Tue, 13 Apr 2021 09:30:23 GMT, Doug Simon wrote: > I guess this should really be named `isJVMCICompilerEnabled` now and the > `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but maybe > that's too big a change (or can be done later). @dougxc, I don't think that we should (or even can) rename it to `vm.jvmcicompiler.enabled`. although the value of the property is indeed `true` whenever a jvmci compiler is enabled, it is used to filter out tests that are incompatible with a particular compiler -- graal. so what we can do is to change `requires/VMProps.java` to set `vm.graal.enabled` only if the jvmci compiler is indeed graal, but on the other hand, we haven't heard anyone complaining that the test coverage for their jvmci compiler has been reduced, so I don't see much of the benefit here. -- Igor - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Tue, 27 Apr 2021 19:07:45 GMT, Igor Ignatyev wrote: > > I guess this should really be named `isJVMCICompilerEnabled` now and the > > `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but > > maybe that's too big a change (or can be done later). > > @dougxc, I don't think that we should (or even can) rename it to > `vm.jvmcicompiler.enabled`. although the value of the property is indeed > `true` whenever a jvmci compiler is enabled, it is used to filter out tests > that are incompatible with a particular compiler -- graal. so what we can do > is to change `requires/VMProps.java` to set `vm.graal.enabled` only if the > jvmci compiler is indeed graal, but on the other hand, we haven't heard > anyone complaining that the test coverage for their jvmci compiler has been > reduced, so I don't see much of the benefit here. > > -- Igor Yes, we should just it as is until a second JVMCI compiler shows up. - PR: https://git.openjdk.java.net/jdk/pull/3421