Skip to content
Permalink
Browse files
8263582: WB_IsMethodCompilable ignores compiler directives
Reviewed-by: iveresov, kvn, neliasso
  • Loading branch information
chhagedorn committed Mar 31, 2021
1 parent 928fa5b commit ab6faa607b81d8ee7641edf251296c9acbc499e3
@@ -112,7 +112,7 @@ void CompilationPolicy::compile_if_required(const methodHandle& m, TRAPS) {
}

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

if (m->is_abstract()) return false;
if (DontCompileHugeMethods && m->code_size() > HugeMethodLimit) return false;
@@ -140,7 +140,7 @@ bool CompilationPolicy::can_be_compiled(const methodHandle& m, int comp_level) {
return false;
}
comp_level = adjust_level_for_compilability_query((CompLevel) comp_level);
if (comp_level == CompLevel_all || is_compile(comp_level)) {
if (comp_level == CompLevel_any || is_compile(comp_level)) {
return !m->is_not_compilable(comp_level);
}
return false;
@@ -150,7 +150,7 @@ bool CompilationPolicy::can_be_compiled(const methodHandle& m, int comp_level) {
bool CompilationPolicy::can_be_osr_compiled(const methodHandle& m, int comp_level) {
bool result = false;
comp_level = adjust_level_for_compilability_query((CompLevel) comp_level);
if (comp_level == CompLevel_all || is_compile(comp_level)) {
if (comp_level == CompLevel_any || is_compile(comp_level)) {
result = !m->is_not_osr_compilable(comp_level);
}
return (result && can_be_compiled(m, comp_level));
@@ -237,7 +237,7 @@ class CompilationPolicy : AllStatic {
static jlong start_time() { return _start_time; }

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

// m is allowed to be compiled
static bool can_be_compiled(const methodHandle& m, int comp_level = CompLevel_all);
static bool can_be_compiled(const methodHandle& m, int comp_level = CompLevel_any);
// m is allowed to be osr compiled
static bool can_be_osr_compiled(const methodHandle& m, int comp_level = CompLevel_all);
static bool can_be_osr_compiled(const methodHandle& m, int comp_level = CompLevel_any);
static bool is_compilation_enabled();

static void do_safepoint_work() { }
@@ -55,8 +55,8 @@ enum MethodCompilation {

// Enumeration to distinguish tiers of compilation
enum CompLevel {
CompLevel_any = -2,
CompLevel_all = -2,
CompLevel_any = -2, // Used for querying the state
CompLevel_all = -2, // Used for changing the state
CompLevel_aot = -1,
CompLevel_none = 0, // Interpreter
CompLevel_simple = 1, // C1
@@ -832,6 +832,25 @@ WB_ENTRY(jboolean, WB_IsMethodCompiled(JNIEnv* env, jobject o, jobject method, j
return (code->is_alive() && !code->is_marked_for_deoptimization());
WB_END

static bool is_excluded_for_compiler(AbstractCompiler* comp, methodHandle& mh) {
if (comp == NULL) {
return true;
}
DirectiveSet* directive = DirectivesStack::getMatchingDirective(mh, comp);
if (directive->ExcludeOption) {
return true;
}
return false;
}

static bool can_be_compiled_at_level(methodHandle& mh, jboolean is_osr, int level) {
if (is_osr) {
return CompilationPolicy::can_be_osr_compiled(mh, level);
} else {
return CompilationPolicy::can_be_compiled(mh, level);
}
}

WB_ENTRY(jboolean, WB_IsMethodCompilable(JNIEnv* env, jobject o, jobject method, jint comp_level, jboolean is_osr))
if (method == NULL || comp_level > CompilationPolicy::highest_compile_level()) {
return false;
@@ -840,11 +859,32 @@ WB_ENTRY(jboolean, WB_IsMethodCompilable(JNIEnv* env, jobject o, jobject method,
CHECK_JNI_EXCEPTION_(env, JNI_FALSE);
MutexLocker mu(Compile_lock);
methodHandle mh(THREAD, Method::checked_resolve_jmethod_id(jmid));
if (is_osr) {
return CompilationPolicy::can_be_osr_compiled(mh, comp_level);
} else {
return CompilationPolicy::can_be_compiled(mh, comp_level);

// The ExcludeOption directive is evaluated lazily upon compilation attempt. If a method was not tried to be compiled by
// a compiler, yet, the method object is not set to be not compilable by that compiler. Thus, evaluate the compiler directive
// to exclude a compilation of 'method'.
if (comp_level == CompLevel_any) {
// Both compilers could have ExcludeOption set. Check all combinations.
bool excluded_c1 = is_excluded_for_compiler(CompileBroker::compiler1(), mh);
bool excluded_c2 = is_excluded_for_compiler(CompileBroker::compiler2(), mh);
if (excluded_c1 && excluded_c2) {
// Compilation of 'method' excluded by both compilers.
return false;
}

if (excluded_c1) {
// C1 only has ExcludeOption set: Check if compilable with C2.
return can_be_compiled_at_level(mh, is_osr, CompLevel_full_optimization);
} else if (excluded_c2) {
// C2 only has ExcludeOption set: Check if compilable with C1.
return can_be_compiled_at_level(mh, is_osr, CompLevel_simple);
}
} else if (comp_level > CompLevel_none && is_excluded_for_compiler(CompileBroker::compiler((int)comp_level), mh)) {
// Compilation of 'method' excluded by compiler used for 'comp_level'.
return false;
}

return can_be_compiled_at_level(mh, is_osr, (int)comp_level);
WB_END

WB_ENTRY(jboolean, WB_IsMethodQueuedForCompilation(JNIEnv* env, jobject o, jobject method))
@@ -0,0 +1,88 @@
/*
* Copyright (c) 2021, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @summary Tests WB::isMethodCompilable(m) in combination with compiler directives that prevent a compilation of m.
* @bug 8263582
* @library /test/lib /
* @build sun.hotspot.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
* -XX:CompileCommand=compileonly,compiler.whitebox.TestMethodCompilableCompilerDirectives::doesNotExist
* compiler.whitebox.TestMethodCompilableCompilerDirectives
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
* -XX:CompileCommand=exclude,compiler.whitebox.TestMethodCompilableCompilerDirectives::*
* compiler.whitebox.TestMethodCompilableCompilerDirectives
*/

package compiler.whitebox;

import jdk.test.lib.Asserts;
import sun.hotspot.WhiteBox;
import java.lang.reflect.Method;

public class TestMethodCompilableCompilerDirectives {
private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox();

// Method too simple for C2 and only C1 compiled.
public static int c1Compiled() {
return 3;
}


// Method first C1 and then C2 compiled.
public static int c2Compiled() {
for (int i = 0; i < 100; i++);
return 3;
}

// WB::isMethodCompilable(m) uses Method::is_not_compilable() to decide if m is compilable. Method::is_not_compilable(), however,
// returns false regardless of any compiler directives if m was not yet tried to be compiled. The compiler directive ExcludeOption
// to prevent a compilation is evaluated lazily and is only applied when a compilation for m is attempted.
// Another problem is that Method::is_not_compilable() only returns true for CompLevel_any if C1 AND C2 cannot compile it.
// This means that a compilation of m must have been attempted for C1 and C2 before WB::isMethodCompilable(m, CompLevel_any) will
// ever return false. This disregards any compiler directives (e.g. compileonly, exclude) that prohibit a compilation of m completely.
// WB::isMethodCompilable() should be aware of the ExcludeOption compiler directives at any point in time.
public static void main(String[] args) throws NoSuchMethodException {
Method c1CompiledMethod = TestMethodCompilableCompilerDirectives.class.getDeclaredMethod("c1Compiled");
Method c2CompiledMethod = TestMethodCompilableCompilerDirectives.class.getDeclaredMethod("c2Compiled");

boolean compilable = WhiteBox.getWhiteBox().isMethodCompilable(c1CompiledMethod);
Asserts.assertFalse(compilable);
for (int i = 0; i < 3000; i++) {
c1Compiled();
}
compilable = WhiteBox.getWhiteBox().isMethodCompilable(c1CompiledMethod);
Asserts.assertFalse(compilable);


compilable = WhiteBox.getWhiteBox().isMethodCompilable(c2CompiledMethod);
Asserts.assertFalse(compilable);
for (int i = 0; i < 3000; i++) {
c2Compiled();
}
compilable = WhiteBox.getWhiteBox().isMethodCompilable(c2CompiledMethod);
Asserts.assertFalse(compilable);
}
}

1 comment on commit ab6faa6

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on ab6faa6 Mar 31, 2021

Please sign in to comment.