Skip to content

Commit ab6faa6

Browse files
committed
8263582: WB_IsMethodCompilable ignores compiler directives
Reviewed-by: iveresov, kvn, neliasso
1 parent 928fa5b commit ab6faa6

File tree

5 files changed

+141
-13
lines changed

5 files changed

+141
-13
lines changed

src/hotspot/share/compiler/compilationPolicy.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ void CompilationPolicy::compile_if_required(const methodHandle& m, TRAPS) {
112112
}
113113

114114
static inline CompLevel adjust_level_for_compilability_query(CompLevel comp_level) {
115-
if (comp_level == CompLevel_all) {
115+
if (comp_level == CompLevel_any) {
116116
if (CompilerConfig::is_c1_only()) {
117117
comp_level = CompLevel_simple;
118118
} else if (CompilerConfig::is_c2_or_jvmci_compiler_only()) {
@@ -125,7 +125,7 @@ static inline CompLevel adjust_level_for_compilability_query(CompLevel comp_leve
125125
// Returns true if m is allowed to be compiled
126126
bool CompilationPolicy::can_be_compiled(const methodHandle& m, int comp_level) {
127127
// allow any levels for WhiteBox
128-
assert(WhiteBoxAPI || comp_level == CompLevel_all || is_compile(comp_level), "illegal compilation level");
128+
assert(WhiteBoxAPI || comp_level == CompLevel_any || is_compile(comp_level), "illegal compilation level");
129129

130130
if (m->is_abstract()) return false;
131131
if (DontCompileHugeMethods && m->code_size() > HugeMethodLimit) return false;
@@ -140,7 +140,7 @@ bool CompilationPolicy::can_be_compiled(const methodHandle& m, int comp_level) {
140140
return false;
141141
}
142142
comp_level = adjust_level_for_compilability_query((CompLevel) comp_level);
143-
if (comp_level == CompLevel_all || is_compile(comp_level)) {
143+
if (comp_level == CompLevel_any || is_compile(comp_level)) {
144144
return !m->is_not_compilable(comp_level);
145145
}
146146
return false;
@@ -150,7 +150,7 @@ bool CompilationPolicy::can_be_compiled(const methodHandle& m, int comp_level) {
150150
bool CompilationPolicy::can_be_osr_compiled(const methodHandle& m, int comp_level) {
151151
bool result = false;
152152
comp_level = adjust_level_for_compilability_query((CompLevel) comp_level);
153-
if (comp_level == CompLevel_all || is_compile(comp_level)) {
153+
if (comp_level == CompLevel_any || is_compile(comp_level)) {
154154
result = !m->is_not_osr_compilable(comp_level);
155155
}
156156
return (result && can_be_compiled(m, comp_level));

src/hotspot/share/compiler/compilationPolicy.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ class CompilationPolicy : AllStatic {
237237
static jlong start_time() { return _start_time; }
238238

239239
// m must be compiled before executing it
240-
static bool must_be_compiled(const methodHandle& m, int comp_level = CompLevel_all);
240+
static bool must_be_compiled(const methodHandle& m, int comp_level = CompLevel_any);
241241
public:
242242
static int c1_count() { return _c1_count; }
243243
static int c2_count() { return _c2_count; }
@@ -248,9 +248,9 @@ class CompilationPolicy : AllStatic {
248248
static void compile_if_required(const methodHandle& m, TRAPS);
249249

250250
// m is allowed to be compiled
251-
static bool can_be_compiled(const methodHandle& m, int comp_level = CompLevel_all);
251+
static bool can_be_compiled(const methodHandle& m, int comp_level = CompLevel_any);
252252
// m is allowed to be osr compiled
253-
static bool can_be_osr_compiled(const methodHandle& m, int comp_level = CompLevel_all);
253+
static bool can_be_osr_compiled(const methodHandle& m, int comp_level = CompLevel_any);
254254
static bool is_compilation_enabled();
255255

256256
static void do_safepoint_work() { }

src/hotspot/share/compiler/compilerDefinitions.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ enum MethodCompilation {
5555

5656
// Enumeration to distinguish tiers of compilation
5757
enum CompLevel {
58-
CompLevel_any = -2,
59-
CompLevel_all = -2,
58+
CompLevel_any = -2, // Used for querying the state
59+
CompLevel_all = -2, // Used for changing the state
6060
CompLevel_aot = -1,
6161
CompLevel_none = 0, // Interpreter
6262
CompLevel_simple = 1, // C1

src/hotspot/share/prims/whitebox.cpp

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,25 @@ WB_ENTRY(jboolean, WB_IsMethodCompiled(JNIEnv* env, jobject o, jobject method, j
832832
return (code->is_alive() && !code->is_marked_for_deoptimization());
833833
WB_END
834834

835+
static bool is_excluded_for_compiler(AbstractCompiler* comp, methodHandle& mh) {
836+
if (comp == NULL) {
837+
return true;
838+
}
839+
DirectiveSet* directive = DirectivesStack::getMatchingDirective(mh, comp);
840+
if (directive->ExcludeOption) {
841+
return true;
842+
}
843+
return false;
844+
}
845+
846+
static bool can_be_compiled_at_level(methodHandle& mh, jboolean is_osr, int level) {
847+
if (is_osr) {
848+
return CompilationPolicy::can_be_osr_compiled(mh, level);
849+
} else {
850+
return CompilationPolicy::can_be_compiled(mh, level);
851+
}
852+
}
853+
835854
WB_ENTRY(jboolean, WB_IsMethodCompilable(JNIEnv* env, jobject o, jobject method, jint comp_level, jboolean is_osr))
836855
if (method == NULL || comp_level > CompilationPolicy::highest_compile_level()) {
837856
return false;
@@ -840,11 +859,32 @@ WB_ENTRY(jboolean, WB_IsMethodCompilable(JNIEnv* env, jobject o, jobject method,
840859
CHECK_JNI_EXCEPTION_(env, JNI_FALSE);
841860
MutexLocker mu(Compile_lock);
842861
methodHandle mh(THREAD, Method::checked_resolve_jmethod_id(jmid));
843-
if (is_osr) {
844-
return CompilationPolicy::can_be_osr_compiled(mh, comp_level);
845-
} else {
846-
return CompilationPolicy::can_be_compiled(mh, comp_level);
862+
863+
// The ExcludeOption directive is evaluated lazily upon compilation attempt. If a method was not tried to be compiled by
864+
// a compiler, yet, the method object is not set to be not compilable by that compiler. Thus, evaluate the compiler directive
865+
// to exclude a compilation of 'method'.
866+
if (comp_level == CompLevel_any) {
867+
// Both compilers could have ExcludeOption set. Check all combinations.
868+
bool excluded_c1 = is_excluded_for_compiler(CompileBroker::compiler1(), mh);
869+
bool excluded_c2 = is_excluded_for_compiler(CompileBroker::compiler2(), mh);
870+
if (excluded_c1 && excluded_c2) {
871+
// Compilation of 'method' excluded by both compilers.
872+
return false;
873+
}
874+
875+
if (excluded_c1) {
876+
// C1 only has ExcludeOption set: Check if compilable with C2.
877+
return can_be_compiled_at_level(mh, is_osr, CompLevel_full_optimization);
878+
} else if (excluded_c2) {
879+
// C2 only has ExcludeOption set: Check if compilable with C1.
880+
return can_be_compiled_at_level(mh, is_osr, CompLevel_simple);
881+
}
882+
} else if (comp_level > CompLevel_none && is_excluded_for_compiler(CompileBroker::compiler((int)comp_level), mh)) {
883+
// Compilation of 'method' excluded by compiler used for 'comp_level'.
884+
return false;
847885
}
886+
887+
return can_be_compiled_at_level(mh, is_osr, (int)comp_level);
848888
WB_END
849889

850890
WB_ENTRY(jboolean, WB_IsMethodQueuedForCompilation(JNIEnv* env, jobject o, jobject method))
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @summary Tests WB::isMethodCompilable(m) in combination with compiler directives that prevent a compilation of m.
27+
* @bug 8263582
28+
* @library /test/lib /
29+
* @build sun.hotspot.WhiteBox
30+
* @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox
31+
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
32+
* -XX:CompileCommand=compileonly,compiler.whitebox.TestMethodCompilableCompilerDirectives::doesNotExist
33+
* compiler.whitebox.TestMethodCompilableCompilerDirectives
34+
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
35+
* -XX:CompileCommand=exclude,compiler.whitebox.TestMethodCompilableCompilerDirectives::*
36+
* compiler.whitebox.TestMethodCompilableCompilerDirectives
37+
*/
38+
39+
package compiler.whitebox;
40+
41+
import jdk.test.lib.Asserts;
42+
import sun.hotspot.WhiteBox;
43+
import java.lang.reflect.Method;
44+
45+
public class TestMethodCompilableCompilerDirectives {
46+
private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox();
47+
48+
// Method too simple for C2 and only C1 compiled.
49+
public static int c1Compiled() {
50+
return 3;
51+
}
52+
53+
54+
// Method first C1 and then C2 compiled.
55+
public static int c2Compiled() {
56+
for (int i = 0; i < 100; i++);
57+
return 3;
58+
}
59+
60+
// WB::isMethodCompilable(m) uses Method::is_not_compilable() to decide if m is compilable. Method::is_not_compilable(), however,
61+
// returns false regardless of any compiler directives if m was not yet tried to be compiled. The compiler directive ExcludeOption
62+
// to prevent a compilation is evaluated lazily and is only applied when a compilation for m is attempted.
63+
// Another problem is that Method::is_not_compilable() only returns true for CompLevel_any if C1 AND C2 cannot compile it.
64+
// This means that a compilation of m must have been attempted for C1 and C2 before WB::isMethodCompilable(m, CompLevel_any) will
65+
// ever return false. This disregards any compiler directives (e.g. compileonly, exclude) that prohibit a compilation of m completely.
66+
// WB::isMethodCompilable() should be aware of the ExcludeOption compiler directives at any point in time.
67+
public static void main(String[] args) throws NoSuchMethodException {
68+
Method c1CompiledMethod = TestMethodCompilableCompilerDirectives.class.getDeclaredMethod("c1Compiled");
69+
Method c2CompiledMethod = TestMethodCompilableCompilerDirectives.class.getDeclaredMethod("c2Compiled");
70+
71+
boolean compilable = WhiteBox.getWhiteBox().isMethodCompilable(c1CompiledMethod);
72+
Asserts.assertFalse(compilable);
73+
for (int i = 0; i < 3000; i++) {
74+
c1Compiled();
75+
}
76+
compilable = WhiteBox.getWhiteBox().isMethodCompilable(c1CompiledMethod);
77+
Asserts.assertFalse(compilable);
78+
79+
80+
compilable = WhiteBox.getWhiteBox().isMethodCompilable(c2CompiledMethod);
81+
Asserts.assertFalse(compilable);
82+
for (int i = 0; i < 3000; i++) {
83+
c2Compiled();
84+
}
85+
compilable = WhiteBox.getWhiteBox().isMethodCompilable(c2CompiledMethod);
86+
Asserts.assertFalse(compilable);
87+
}
88+
}

0 commit comments

Comments
 (0)