Add a test and CHECKs around the combination of CHA and default methods.
Test: 823-cha-inlining
Bug: 182538502
Change-Id: Ie3e0014804216802af0addf13751a8f89adbfdfa
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 96cb605..3e9969d 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1286,11 +1286,14 @@
return false;
}
- if (method->IsDefault() && method->IsDefaultConflicting()) {
- // Changing to invoke-virtual cannot be done on default conflict method
- // since it's not in any vtable.
- DCHECK(cha_devirtualize);
- return false;
+ if (kIsDebugBuild && method->IsDefaultConflicting()) {
+ CHECK(!cha_devirtualize) << "CHA cannot have a default conflict method as target";
+ // Devirtualization by exact type/inline-cache always uses a method in the vtable,
+ // so it's OK to change this invoke into a HInvokeVirtual.
+ ObjPtr<mirror::Class> receiver_class = receiver_type.GetTypeHandle().Get();
+ CHECK(!receiver_class->IsInterface());
+ PointerSize pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize();
+ CHECK(method == receiver_class->GetVTableEntry(method->GetMethodIndex(), pointer_size));
}
uint32_t dex_method_index = FindMethodIndexIn(
@@ -1786,14 +1789,12 @@
bool HInliner::CanInlineBody(const HGraph* callee_graph,
const HBasicBlock* target_block,
size_t* out_number_of_instructions) const {
- const DexFile& callee_dex_file = callee_graph->GetDexFile();
ArtMethod* const resolved_method = callee_graph->GetArtMethod();
- const uint32_t method_index = resolved_method->GetMethodIndex();
HBasicBlock* exit_block = callee_graph->GetExitBlock();
if (exit_block == nullptr) {
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedInfiniteLoop)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because it has an infinite loop";
return false;
}
@@ -1804,21 +1805,21 @@
if (target_block->IsTryBlock()) {
// TODO(ngeoffray): Support adding HTryBoundary in Hgraph::InlineInto.
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedTryCatch)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because one branch always throws and"
<< " caller is in a try/catch block";
return false;
} else if (graph_->GetExitBlock() == nullptr) {
// TODO(ngeoffray): Support adding HExit in the caller graph.
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedInfiniteLoop)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because one branch always throws and"
<< " caller does not have an exit block";
return false;
} else if (graph_->HasIrreducibleLoops()) {
// TODO(ngeoffray): Support re-computing loop information to graphs with
// irreducible loops?
- VLOG(compiler) << "Method " << callee_dex_file.PrettyMethod(method_index)
+ VLOG(compiler) << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because one branch always throws and"
<< " caller has irreducible loops";
return false;
@@ -1830,7 +1831,7 @@
if (!has_one_return) {
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedAlwaysThrows)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because it always throws";
return false;
}
@@ -1843,7 +1844,7 @@
// Don't inline methods with irreducible loops, they could prevent some
// optimizations to run.
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedIrreducibleLoop)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because it contains an irreducible loop";
return false;
}
@@ -1852,7 +1853,7 @@
// loop information to be computed incorrectly when updating after
// inlining.
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedLoopWithoutExit)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because it contains a loop with no exit";
return false;
}
@@ -1863,7 +1864,7 @@
instr_it.Advance()) {
if (++number_of_instructions >= inlining_budget_) {
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedInstructionBudget)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " is not inlined because the outer method has reached"
<< " its instruction budget limit.";
return false;
@@ -1872,7 +1873,7 @@
if (current->NeedsEnvironment() &&
(total_number_of_dex_registers_ >= kMaximumNumberOfCumulatedDexRegisters)) {
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedEnvironmentBudget)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " is not inlined because its caller has reached"
<< " its environment budget limit.";
return false;
@@ -1882,7 +1883,7 @@
!CanEncodeInlinedMethodInStackMap(*caller_compilation_unit_.GetDexFile(),
resolved_method)) {
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedStackMaps)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because " << current->DebugName()
<< " needs an environment, is in a different dex file"
<< ", and cannot be encoded in the stack maps.";
@@ -1895,7 +1896,7 @@
current->IsUnresolvedInstanceFieldSet()) {
// Entrypoint for unresolved fields does not handle inlined frames.
LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedUnresolvedEntrypoint)
- << "Method " << callee_dex_file.PrettyMethod(method_index)
+ << "Method " << resolved_method->PrettyMethod()
<< " could not be inlined because it is using an unresolved"
<< " entrypoint";
return false;
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index a2db0f0..89dc93b 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -94,11 +94,14 @@
}
ArtMethod* ArtMethod::GetSingleImplementation(PointerSize pointer_size) {
- if (!IsAbstract()) {
- // A non-abstract's single implementation is itself.
+ if (IsInvokable()) {
+ // An invokable method single implementation is itself.
return this;
}
- return reinterpret_cast<ArtMethod*>(GetDataPtrSize(pointer_size));
+ DCHECK(!IsDefaultConflicting());
+ ArtMethod* m = reinterpret_cast<ArtMethod*>(GetDataPtrSize(pointer_size));
+ CHECK(m == nullptr || !m->IsDefaultConflicting());
+ return m;
}
ArtMethod* ArtMethod::FromReflectedMethod(const ScopedObjectAccessAlreadyRunnable& soa,
@@ -736,9 +739,9 @@
image_pointer_size);
}
}
- if (interpreter::IsNterpSupported() &&
- (GetEntryPointFromQuickCompiledCodePtrSize(image_pointer_size) ==
- interpreter::GetNterpEntryPoint())) {
+ if (interpreter::IsNterpSupported() &&
+ (GetEntryPointFromQuickCompiledCodePtrSize(image_pointer_size) ==
+ interpreter::GetNterpEntryPoint())) {
// If the entrypoint is nterp, it's too early to check if the new method
// will support it. So for simplicity, use the interpreter bridge.
SetEntryPointFromQuickCompiledCodePtrSize(GetQuickToInterpreterBridge(), image_pointer_size);
diff --git a/test/823-cha-inlining/expected-stderr.txt b/test/823-cha-inlining/expected-stderr.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/823-cha-inlining/expected-stderr.txt
diff --git a/test/823-cha-inlining/expected-stdout.txt b/test/823-cha-inlining/expected-stdout.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/823-cha-inlining/expected-stdout.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/823-cha-inlining/info.txt b/test/823-cha-inlining/info.txt
new file mode 100644
index 0000000..312a2ef
--- /dev/null
+++ b/test/823-cha-inlining/info.txt
@@ -0,0 +1 @@
+Tests for the combination of CHA and default methods.
diff --git a/test/823-cha-inlining/src/Main.java b/test/823-cha-inlining/src/Main.java
new file mode 100644
index 0000000..e25047d
--- /dev/null
+++ b/test/823-cha-inlining/src/Main.java
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+interface Itf {
+ // We make the methods below directly throw instead of using the $noinline$
+ // directive to get the inliner actually try to inline but decide not to.
+ // This will then make the compiler try to generate an HInvokeVirtual instead
+ // of an HInvokeInterface.
+
+ public default void m() throws Exception {
+ throw new Exception("Don't inline me");
+ }
+ public default void mConflict() throws Exception {
+ throw new Exception("Don't inline me");
+ }
+}
+
+// This is redefined in src2 with a mConflict method.
+interface Itf2 {
+}
+
+interface Itf3 extends Itf, Itf2 {
+}
+
+class Itf3Impl implements Itf3 {
+}
+
+interface Itf4 extends Itf, Itf2 {
+ public default void m() throws Exception {
+ throw new Exception("Don't inline me");
+ }
+}
+
+class Itf4Impl implements Itf4 {
+}
+
+
+public class Main implements Itf, Itf2 {
+
+ public static void main(String[] args) {
+ System.loadLibrary(args[0]);
+
+ // Execute enough time to populate inline caches.
+ for (int i = 0; i < 100000; ++i) {
+ try {
+ $noinline$doCallDefault();
+ } catch (Exception e) {
+ // Expected.
+ }
+ }
+ ensureJitCompiled(Main.class, "$noinline$doCallDefault");
+ try {
+ $noinline$doCallDefault();
+ throw new Error("Expected exception");
+ } catch (Exception e) {
+ // Expected.
+ }
+
+ ensureJitCompiled(Main.class, "$noinline$doCallDefaultConflict");
+ try {
+ $noinline$doCallDefaultConflict();
+ throw new Error("Expected IncompatibleClassChangeError");
+ } catch (Exception e) {
+ throw new Error("Unexpected exception");
+ } catch (IncompatibleClassChangeError e) {
+ // Expected.
+ }
+
+ // Execute enough time to populate inline caches.
+ for (int i = 0; i < 100000; ++i) {
+ try {
+ $noinline$doCallDefaultConflictItf3();
+ } catch (Throwable t) {
+ // Expected.
+ }
+ }
+ ensureJitCompiled(Main.class, "$noinline$doCallDefaultConflictItf3");
+ try {
+ $noinline$doCallDefaultConflictItf3();
+ throw new Error("Expected IncompatibleClassChangeError");
+ } catch (Exception e) {
+ throw new Error("Unexpected exception");
+ } catch (IncompatibleClassChangeError e) {
+ // Expected.
+ }
+
+ ensureJitCompiled(Main.class, "$noinline$doCallDefaultConflictItf4");
+ try {
+ $noinline$doCallDefaultConflictItf4();
+ throw new Error("Expected IncompatibleClassChangeError");
+ } catch (Exception e) {
+ throw new Error("Unexpected exception");
+ } catch (IncompatibleClassChangeError e) {
+ // Expected.
+ }
+ }
+
+ public static void $noinline$doCallDefault() throws Exception {
+ itf.m();
+ }
+
+ public static void $noinline$doCallDefaultConflict() throws Exception {
+ itf.mConflict();
+ }
+
+ public static void $noinline$doCallDefaultConflictItf3() throws Exception {
+ itf3.mConflict();
+ }
+
+ public static void $noinline$doCallDefaultConflictItf4() throws Exception {
+ itf4.mConflict();
+ }
+
+ static Itf itf = new Main();
+ static Itf3 itf3 = new Itf3Impl();
+ static Itf4 itf4 = new Itf4Impl();
+
+ private static native void ensureJitCompiled(Class<?> cls, String methodName);
+}
diff --git a/test/823-cha-inlining/src2/Itf2.java b/test/823-cha-inlining/src2/Itf2.java
new file mode 100644
index 0000000..6b019c4
--- /dev/null
+++ b/test/823-cha-inlining/src2/Itf2.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+interface Itf2 {
+ public default void mConflict() throws Exception {
+ throw new Exception("Don't inline me");
+ }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 7f35091..d4b46ff 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1008,6 +1008,7 @@
"804-class-extends-itself",
"816-illegal-new-array",
"819-verification-runtime",
+ "823-cha-inlining",
"900-hello-plugin",
"901-hello-ti-agent",
"903-hello-tagging",