8264079: Improve abstractions
Reviewed-by: yan
diff --git a/src/hotspot/share/code/dependencies.cpp b/src/hotspot/share/code/dependencies.cpp
index 8839eba..746dd18 100644
--- a/src/hotspot/share/code/dependencies.cpp
+++ b/src/hotspot/share/code/dependencies.cpp
@@ -1301,9 +1301,9 @@
// the spot-checking version:
Klass* find_witness_in(KlassDepChange& changes,
Klass* context_type,
- bool participants_hide_witnesses);
- bool witnessed_reabstraction_in_supers(Klass* k);
+ bool participants_hide_witnesses);
public:
+ bool witnessed_reabstraction_in_supers(Klass* k);
Klass* find_witness_subtype(Klass* context_type, KlassDepChange* changes = NULL) {
assert(doing_subtype_search(), "must set up a subtype search");
// When looking for unexpected concrete types,
@@ -1414,15 +1414,8 @@
}
}
- if (is_witness(new_type)) {
- if (!ignore_witness(new_type)) {
- return new_type;
- }
- } else if (!doing_subtype_search()) {
- // No witness found, but is_witness() doesn't detect method re-abstraction in case of spot-checking.
- if (witnessed_reabstraction_in_supers(new_type)) {
- return new_type;
- }
+ if (is_witness(new_type) && !ignore_witness(new_type)) {
+ return new_type;
}
return NULL;
@@ -1804,18 +1797,91 @@
return num;
}
+
+// Try to determine whether root method in some context is concrete or not based on the information about the unique method
+// in that context. It exploits the fact that concrete root method is always inherited into the context when there's a unique method.
+// Hence, unique method holder is always a supertype of the context class when root method is concrete.
+// Examples for concrete_root_method
+// C (C.m uniqm)
+// |
+// CX (ctxk) uniqm is inherited into context.
+//
+// CX (ctxk) (CX.m uniqm) here uniqm is defined in ctxk.
+// Examples for !concrete_root_method
+// CX (ctxk)
+// |
+// C (C.m uniqm) uniqm is in subtype of ctxk.
+bool Dependencies::is_concrete_root_method(Method* uniqm, Klass* ctxk) {
+ if (uniqm == NULL) {
+ return false; // match Dependencies::is_concrete_method() behavior
+ }
+ // Theoretically, the "direction" of subtype check matters here.
+ // On one hand, in case of interface context with a single implementor, uniqm can be in a superclass of the implementor which
+ // is not related to context class.
+ // On another hand, uniqm could come from an interface unrelated to the context class, but right now it is not possible:
+ // it is required that uniqm->method_holder() is the participant (uniqm->method_holder() <: ctxk), hence a default method
+ // can't be used as unique.
+ if (ctxk->is_interface()) {
+ Klass* implementor = InstanceKlass::cast(ctxk)->implementor();
+ assert(implementor != ctxk, "single implementor only"); // should have been invalidated earlier
+ ctxk = implementor;
+ }
+ InstanceKlass* holder = uniqm->method_holder();
+ assert(!holder->is_interface(), "no default methods allowed");
+ assert(ctxk->is_subclass_of(holder) || holder->is_subclass_of(ctxk), "not related");
+ return ctxk->is_subclass_of(holder);
+}
+
// If a class (or interface) has a unique concrete method uniqm, return NULL.
// Otherwise, return a class that contains an interfering method.
-Klass* Dependencies::check_unique_concrete_method(Klass* ctxk, Method* uniqm,
- KlassDepChange* changes) {
- // Here is a missing optimization: If uniqm->is_final(),
- // we don't really need to search beneath it for overrides.
- // This is probably not important, since we don't use dependencies
- // to track final methods. (They can't be "definalized".)
+Klass* Dependencies::check_unique_concrete_method(Klass* ctxk,
+ Method* uniqm,
+ KlassDepChange* changes) {
ClassHierarchyWalker wf(uniqm->method_holder(), uniqm);
- return wf.find_witness_definer(ctxk, changes);
+ Klass* witness = wf.find_witness_definer(ctxk, changes);
+ if (witness != NULL) {
+ return witness;
+ }
+ if (!Dependencies::is_concrete_root_method(uniqm, ctxk) || changes != NULL) {
+ Klass* conck = find_witness_AME(ctxk, uniqm, changes);
+ if (conck != NULL) {
+ // Found a concrete subtype 'conck' which does not override abstract root method.
+ return conck;
+ }
+ }
+ return NULL;
}
+// Search for AME.
+// There are two version of checks.
+// 1) Spot checking version(Classload time). Newly added class is checked for AME.
+// Checks whether abstract/overpass method is inherited into/declared in newly added concrete class.
+// 2) Compile time analysis for abstract/overpass(abstract klass) root_m. The non uniqm subtrees are checked for concrete classes.
+Klass* Dependencies::find_witness_AME(Klass* ctxk, Method* m, KlassDepChange* changes) {
+ if (m != NULL) {
+ if (changes != NULL) {
+ // Spot checking version.
+ ClassHierarchyWalker wf(m);
+ Klass* new_type = changes->new_type();
+ if (wf.witnessed_reabstraction_in_supers(new_type)) {
+ return new_type;
+ }
+ } else {
+ // Note: It is required that uniqm->method_holder() is the participant (see ClassHierarchyWalker::found_method()).
+ ClassHierarchyWalker wf(m->method_holder());
+ Klass* conck = wf.find_witness_subtype(ctxk);
+ if (conck != NULL) {
+ Method* cm = InstanceKlass::cast(conck)->find_instance_method(m->name(), m->signature(), Klass::skip_private);
+ if (!Dependencies::is_concrete_method(cm, conck)) {
+ return conck;
+ }
+ }
+ }
+ }
+ return NULL;
+}
+
+
// Find the set of all non-abstract methods under ctxk that match m.
// (The method m must be defined or inherited in ctxk.)
// Include m itself in the set, unless it is abstract.
@@ -1840,7 +1906,11 @@
// (This can happen if m is inherited into ctxk and fm overrides it.)
return NULL;
}
+ } else if (Dependencies::find_witness_AME(ctxk, fm) != NULL) {
+ // Found a concrete subtype which does not override abstract root method.
+ return NULL;
}
+ assert(Dependencies::is_concrete_root_method(fm, ctxk) == Dependencies::is_concrete_method(m, ctxk), "mismatch");
#ifndef PRODUCT
// Make sure the dependency mechanism will pass this discovery:
if (VerifyDependencies && fm != NULL) {
diff --git a/src/hotspot/share/code/dependencies.hpp b/src/hotspot/share/code/dependencies.hpp
index 07c17a2..52409dc 100644
--- a/src/hotspot/share/code/dependencies.hpp
+++ b/src/hotspot/share/code/dependencies.hpp
@@ -393,6 +393,9 @@
static bool is_concrete_method(Method* m, Klass* k); // m is invocable
static Klass* find_finalizable_subclass(Klass* k);
+ static bool is_concrete_root_method(Method* uniqm, Klass* ctxk);
+ static Klass* find_witness_AME(Klass* ctxk, Method* m, KlassDepChange* changes = NULL);
+
// These versions of the concreteness queries work through the CI.
// The CI versions are allowed to skew sometimes from the VM
// (oop-based) versions. The cost of such a difference is a
diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
index f102b79..99caa1c 100644
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
@@ -367,10 +367,6 @@
THROW_MSG_0(vmSymbols::java_lang_InternalError(), err_msg("Interface %s should be handled in Java code", holder->external_name()));
}
- if (method->is_abstract()) {
- return NULL;
- }
-
methodHandle ucm;
{
MutexLocker locker(Compile_lock);
diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/MarkUnsafeAccessTest.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/MarkUnsafeAccessTest.java
index f496522..d95a683 100644
--- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/MarkUnsafeAccessTest.java
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/MarkUnsafeAccessTest.java
@@ -116,8 +116,6 @@
testMappedByteBuffer(MappedByteBuffer::get);
}
- // TODO Re-enable this test once JDK-8259360 got fixed.
- /*
@Test
public void testCompiled() throws IOException {
Assume.assumeFalse("Crashes on AArch64 (GR-8351)", System.getProperty("os.arch").equalsIgnoreCase("aarch64"));
@@ -139,7 +137,6 @@
}
});
}
- */
private static final int BLOCK_SIZE = 512;
private static final int BLOCK_COUNT = 16;
