RFR: 8251462: Remove legacy compilation policy [v4]

Vladimir Kozlov kvn at openjdk.java.net
Fri Jan 22 01:48:46 UTC 2021


On Tue, 12 Jan 2021 05:04:11 GMT, Igor Veresov <iveresov at openjdk.org> wrote:

>> This change removes the legacy compilation policy and an emulation mode to the tiered policy to simulate the old behavior with ```-XX:-TieredCompilation```. The change removed a bunch of interpreter code, devirtualizes the compilation policy API, adds a consistent way to query compiler configuration with the new ```CompilerConfig``` API.
>> 
>> I've tested this with hs-tier{1,2,3,4,5}. And also made sure it builds and works with C1/C2-Graal/AOT being enabled/disabled.
>> 
>> Since there are platform-specific changes I would greatly appreciate some help from the maintainers of the specific ports to verify the build and run basic smoke tests. I've already tested x64 and aarch64. Thanks!
>
> Igor Veresov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Check legacy flags validity before deriving flag values for emulation mode.

I would also suggest to do performance runs. Ask Eric for help. Changes are significant and may affect performance due to some typo.

src/hotspot/share/compiler/compilerDefinitions.hpp line 234:

> 232:   static bool is_interpreter_only() {
> 233:     return Arguments::is_interpreter_only() || TieredStopAtLevel == CompLevel_none;
> 234:   }

Can you move these functions after has_*() functions? They are used before.

src/hotspot/share/compiler/compilerDefinitions.hpp line 179:

> 177:   }
> 178:   // Is the JVM in a configuration that permits only c2-compiled methods?
> 179:   // JVMCI compiler replaces C2.

The comment `JVMCI compiler replaces C2.` should be removed or moved to `is_jvmci_compiler_only()` where it is make more sense.

src/hotspot/share/compiler/compilerDefinitions.hpp line 171:

> 169:   }
> 170: 
> 171:   static bool is_c2_available() {

For me `_available` suffix sounds similar to `has_`. May be `_enabled` is better. At least it is less confusing where it is called.

src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp line 1414:

> 1412:   // use LD;DMB but stores use STLR.  This can happen if C2 compiles
> 1413:   // the stores in one method and C1 compiles the loads in another.
> 1414:   if (!CompilerConfig::is_c1_or_interpreter_only_no_aot_or_jvmci()) {

It is C1 code which should not be executed in -Xint. Why check `interpreter_only`?

src/hotspot/cpu/aarch64/gc/shenandoah/c1/shenandoahBarrierSetC1_aarch64.cpp line 54:

> 52:   ShenandoahBarrierSet::assembler()->cmpxchg_oop(masm->masm(), addr, cmpval, newval, /*acquire*/ true, /*release*/ true, /*is_cae*/ false, result);
> 53: 
> 54:   if (CompilerConfig::is_c1_or_interpreter_only_no_aot_or_jvmci()) {

Again about `interpreter_only` check.

src/hotspot/share/compiler/compilerDefinitions.hpp line 243:

> 241:   static bool is_c1_only_no_aot_or_jvmci() {
> 242:     return is_c1_only() && !is_aot() && !is_jvmci();
> 243:   }

These names are a little confusing: what about C2, why only no AOT and JVMCI. I understand that you want to check if JVMCI or AOT can install their compiled code.
May be `is_c1_nmethods_only`, `is_c1_nmethods_or_interpreter_only` ?

src/hotspot/share/oops/method.cpp line 1013:

> 1011:     return false;
> 1012:   if (comp_level == CompLevel_any)
> 1013:     return is_not_c1_compilable() && is_not_c2_compilable();

Was it bug before?

src/hotspot/share/compiler/compilerDefinitions.cpp line 62:

> 60:       }
> 61:     } else if (strcmp(CompilationMode, "high-only") == 0) {
> 62:       if (!CompilerConfig::has_c2() && !CompilerConfig::is_jvmci_compiler()) {

Is using `!is_c2_or_jvmci_compiler_available()` better here? All flags should be processed at this point. And we could have some `TieredStopAtLevel` flags.
It looks like you have cross dependency between CompilerConfig and CompilationModeFlag which make using `is_c2_or_jvmci_compiler_available()` impossible here.

src/hotspot/share/compiler/compilerDefinitions.cpp line 84:

> 82:     } else if (CompilerConfig::is_c2_or_jvmci_compiler_only()) {
> 83:       _mode = Mode::HIGH_ONLY;
> 84:     } else if (CompilerConfig::is_jvmci_compiler() && !TieredCompilation) {

Should you check `CompilerConfig::is_tiered()` instead of `TieredCompilation` flag?

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

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


More information about the hotspot-compiler-dev mailing list