Revert "Devirtualize to HInvokeStaticOrDirect."
This reverts commit 5024ddfd125b5c3b59d7f359ae33cf7f0255b048.
Bug: 187408838
Reason for revert: b/187408838
Change-Id: If74f5ddbacc73296f66c55762e2a8d1ec2cd1f19
diff --git a/TEST_MAPPING b/TEST_MAPPING
index d869991..7848ec2 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -530,6 +530,9 @@
"name": "art-run-test-449-checker-bce-rem[com.google.android.art.apex]"
},
{
+ "name": "art-run-test-450-checker-types[com.google.android.art.apex]"
+ },
+ {
"name": "art-run-test-451-regression-add-float[com.google.android.art.apex]"
},
{
@@ -1384,6 +1387,9 @@
"name": "art-run-test-449-checker-bce-rem"
},
{
+ "name": "art-run-test-450-checker-types"
+ },
+ {
"name": "art-run-test-451-regression-add-float"
},
{
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index f251817..611c691 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1240,22 +1240,32 @@
bool HInliner::TryDevirtualize(HInvoke* invoke_instruction,
ArtMethod* method,
HInvoke** replacement) {
+ DCHECK(!method->IsProxyMethod());
DCHECK(invoke_instruction != *replacement);
- if (!invoke_instruction->IsInvokeInterface() && !invoke_instruction->IsInvokeVirtual()) {
+ if (!invoke_instruction->IsInvokeInterface()) {
+ // TODO: Consider sharpening an invoke virtual once it is not dependent on the
+ // compiler driver.
return false;
}
+ // Devirtualization by exact type uses a method in the vtable, so we should
+ // not see a default non-copied method.
+ DCHECK(!method->IsDefault() || method->IsCopied());
+ // Turn an invoke-interface into an invoke-virtual. An invoke-virtual is always
+ // better than an invoke-interface because:
+ // 1) In the best case, the interface call has one more indirection (to fetch the IMT).
+ // 2) We will not go to the conflict trampoline with an invoke-virtual.
+ // TODO: Consider sharpening once it is not dependent on the compiler driver.
+
+ if (kIsDebugBuild && method->IsDefaultConflicting()) {
+ ReferenceTypeInfo receiver_type = invoke_instruction->InputAt(0)->GetReferenceTypeInfo();
+ // Devirtualization by exact type 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));
+ }
- // Don't bother trying to call directly a default conflict method. It
- // doesn't have a proper MethodReference, but also `GetCanonicalMethod`
- // will return an actual default implementation.
- if (method->IsDefaultConflicting()) {
- return false;
- }
- DCHECK(!method->IsProxyMethod());
- ClassLinker* cl = Runtime::Current()->GetClassLinker();
- PointerSize pointer_size = cl->GetImagePointerSize();
- // The sharpening logic assumes the caller isn't passing a copied method.
- method = method->GetCanonicalMethod(pointer_size);
uint32_t dex_method_index = FindMethodIndexIn(
method,
*invoke_instruction->GetMethodReference().dex_file,
@@ -1263,32 +1273,19 @@
if (dex_method_index == dex::kDexNoIndex) {
return false;
}
- HInvokeStaticOrDirect::DispatchInfo dispatch_info =
- HSharpening::SharpenLoadMethod(method,
- /* has_method_id= */ true,
- /* for_interface_call= */ false,
- codegen_);
- DCHECK_NE(dispatch_info.code_ptr_location, CodePtrLocation::kCallCriticalNative);
- HInvokeStaticOrDirect* new_invoke = new (graph_->GetAllocator()) HInvokeStaticOrDirect(
+ HInvokeVirtual* new_invoke = new (graph_->GetAllocator()) HInvokeVirtual(
graph_->GetAllocator(),
invoke_instruction->GetNumberOfArguments(),
invoke_instruction->GetType(),
invoke_instruction->GetDexPc(),
MethodReference(invoke_instruction->GetMethodReference().dex_file, dex_method_index),
method,
- dispatch_info,
- kDirect,
MethodReference(method->GetDexFile(), method->GetDexMethodIndex()),
- HInvokeStaticOrDirect::ClinitCheckRequirement::kNone);
+ method->GetMethodIndex());
HInputsRef inputs = invoke_instruction->GetInputs();
- DCHECK_EQ(inputs.size(), invoke_instruction->GetNumberOfArguments());
for (size_t index = 0; index != inputs.size(); ++index) {
new_invoke->SetArgumentAt(index, inputs[index]);
}
- if (HInvokeStaticOrDirect::NeedsCurrentMethodInput(dispatch_info)) {
- new_invoke->SetRawInputAt(new_invoke->GetCurrentMethodIndexUnchecked(),
- graph_->GetCurrentMethod());
- }
invoke_instruction->GetBlock()->InsertInstructionBefore(new_invoke, invoke_instruction);
new_invoke->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
if (invoke_instruction->GetType() == DataType::Type::kReference) {
diff --git a/runtime/entrypoints/entrypoint_utils.cc b/runtime/entrypoints/entrypoint_utils.cc
index 1c0127a..ef0c474 100644
--- a/runtime/entrypoints/entrypoint_utils.cc
+++ b/runtime/entrypoints/entrypoint_utils.cc
@@ -302,10 +302,6 @@
oat_file->GetBssMethods().data() + oat_file->GetBssMethods().size());
std::atomic<ArtMethod*>* atomic_entry =
reinterpret_cast<std::atomic<ArtMethod*>*>(method_entry);
- if (kIsDebugBuild) {
- ArtMethod* existing = atomic_entry->load(std::memory_order_acquire);
- CHECK(existing->IsRuntimeMethod() || existing == callee);
- }
static_assert(sizeof(*method_entry) == sizeof(*atomic_entry), "Size check.");
atomic_entry->store(callee, std::memory_order_release);
}
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index eaa9f4e..bc5a10d 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1357,7 +1357,7 @@
// odd situation where the ArtMethod being executed is unrelated to the
// receiver of the method.
called = called->GetCanonicalMethod();
- if (invoke_type == kSuper || invoke_type == kInterface || invoke_type == kVirtual) {
+ if (invoke_type == kSuper) {
if (called->GetDexFile() == called_method.dex_file) {
called_method.index = called->GetDexMethodIndex();
} else {
diff --git a/test/450-checker-types/Android.bp b/test/450-checker-types/Android.bp
index 1173b5a..bf45226 100644
--- a/test/450-checker-types/Android.bp
+++ b/test/450-checker-types/Android.bp
@@ -15,7 +15,7 @@
java_test {
name: "art-run-test-450-checker-types",
defaults: ["art-run-test-defaults"],
- test_config_template: ":art-run-test-target-no-test-suite-tag-template",
+ test_config_template: ":art-run-test-target-template",
srcs: ["src/**/*.java"],
data: [
":art-run-test-450-checker-types-expected-stdout",
diff --git a/test/450-checker-types/src/Main.java b/test/450-checker-types/src/Main.java
index 084a713..9e3f951 100644
--- a/test/450-checker-types/src/Main.java
+++ b/test/450-checker-types/src/Main.java
@@ -686,7 +686,7 @@
/// CHECK-DAG: <<Null:l\d+>> NullConstant
/// CHECK-DAG: <<Phi:l\d+>> Phi [<<Arg>>,<<Null>>] klass:SubclassA
/// CHECK-DAG: <<NCPhi:l\d+>> NullCheck [<<Phi>>]
- /// CHECK-DAG: InvokeStaticOrDirect [<<NCPhi>>] method_name:java.lang.Object.hashCode
+ /// CHECK-DAG: InvokeVirtual [<<NCPhi>>] method_name:java.lang.Object.hashCode
public void testThisArgumentMoreSpecific(boolean cond) {
// Inlining method from Super will build it with `this` typed as Super.
diff --git a/test/569-checker-pattern-replacement/src/Main.java b/test/569-checker-pattern-replacement/src/Main.java
index e2658a4..bcf8cdd 100644
--- a/test/569-checker-pattern-replacement/src/Main.java
+++ b/test/569-checker-pattern-replacement/src/Main.java
@@ -209,11 +209,10 @@
/// CHECK: {{d\d+}} InvokeVirtual
/// CHECK-START: double Main.getDoubleFromParam(Second) inliner (after)
- /// CHECK: {{d\d+}} InvokeStaticOrDirect
+ /// CHECK: {{d\d+}} InvokeVirtual
/// CHECK-START: double Main.getDoubleFromParam(Second) inliner (after)
/// CHECK-NOT: InstanceFieldGet
- /// CHECK-NOT: InvokeVirtual
public static double getDoubleFromParam(Second s) {
return s.getInstanceDoubleFieldFromParam(s);
@@ -223,12 +222,11 @@
/// CHECK: {{i\d+}} InvokeVirtual
/// CHECK-START: int Main.getStaticInt(Second) inliner (after)
- /// CHECK: {{i\d+}} InvokeStaticOrDirect
+ /// CHECK: {{i\d+}} InvokeVirtual
/// CHECK-START: int Main.getStaticInt(Second) inliner (after)
/// CHECK-NOT: InstanceFieldGet
/// CHECK-NOT: StaticFieldGet
- /// CHECK-NOT: InvokeVirtual
public static int getStaticInt(Second s) {
return s.getStaticIntField();
@@ -289,11 +287,10 @@
/// CHECK: InvokeVirtual
/// CHECK-START: long Main.setLongThroughParam(Second, long) inliner (after)
- /// CHECK: InvokeStaticOrDirect
+ /// CHECK: InvokeVirtual
/// CHECK-START: long Main.setLongThroughParam(Second, long) inliner (after)
/// CHECK-NOT: InstanceFieldSet
- /// CHECK-NOT: InvokeVirtual
public static long setLongThroughParam(Second s, long value) {
s.setInstanceLongFieldThroughParam(s, value);
@@ -304,12 +301,11 @@
/// CHECK: InvokeVirtual
/// CHECK-START: float Main.setStaticFloat(Second, float) inliner (after)
- /// CHECK: InvokeStaticOrDirect
+ /// CHECK: InvokeVirtual
/// CHECK-START: float Main.setStaticFloat(Second, float) inliner (after)
/// CHECK-NOT: InstanceFieldSet
/// CHECK-NOT: StaticFieldSet
- /// CHECK-NOT: InvokeVirtual
public static float setStaticFloat(Second s, float value) {
s.setStaticFloatField(value);
diff --git a/test/609-checker-inline-interface/src/Main.java b/test/609-checker-inline-interface/src/Main.java
index 07709ea..249b778 100644
--- a/test/609-checker-inline-interface/src/Main.java
+++ b/test/609-checker-inline-interface/src/Main.java
@@ -32,7 +32,7 @@
// Expected
}
try {
- testInterfaceToDirectCall();
+ testInterfaceToVirtualCall();
} catch (Error e) {
// Expected.
}
@@ -53,19 +53,19 @@
methodWithInvokeInterface(itf);
}
- /// CHECK-START: void Main.testInterfaceToDirectCall() inliner (before)
+ /// CHECK-START: void Main.testInterfaceToVirtualCall() inliner (before)
/// CHECK: InvokeStaticOrDirect method_name:Main.methodWithInvokeInterface
- /// CHECK-START: void Main.testInterfaceToDirectCall() inliner (before)
+ /// CHECK-START: void Main.testInterfaceToVirtualCall() inliner (before)
/// CHECK-NOT: InvokeInterface
- /// CHECK-START: void Main.testInterfaceToDirectCall() inliner (after)
- /// CHECK: InvokeStaticOrDirect method_name:Main.doCall
+ /// CHECK-START: void Main.testInterfaceToVirtualCall() inliner (after)
+ /// CHECK: InvokeVirtual method_name:Main.doCall
- /// CHECK-START: void Main.testInterfaceToDirectCall() inliner (after)
- /// CHECK-NOT: InvokeVirtual
+ /// CHECK-START: void Main.testInterfaceToVirtualCall() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
/// CHECK-NOT: InvokeInterface
- public static void testInterfaceToDirectCall() {
+ public static void testInterfaceToVirtualCall() {
methodWithInvokeInterface(m);
}
diff --git a/test/673-checker-throw-vmethod/src/Main.java b/test/673-checker-throw-vmethod/src/Main.java
index 3979300..f62cfc8 100644
--- a/test/673-checker-throw-vmethod/src/Main.java
+++ b/test/673-checker-throw-vmethod/src/Main.java
@@ -81,7 +81,7 @@
/// CHECK: If [<<Tst>>]
/// CHECK: end_block
/// CHECK: begin_block
- /// CHECK: InvokeStaticOrDirect [{{l\d+}},<<Str>>] method_name:Main.doThrow
+ /// CHECK: InvokeVirtual [{{l\d+}},<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
//
/// CHECK-START: void Main.doit2(int[]) code_sinking (after)
@@ -91,7 +91,7 @@
/// CHECK: end_block
/// CHECK: begin_block
/// CHECK: <<Str:l\d+>> LoadString
- /// CHECK: InvokeStaticOrDirect [{{l\d+}},<<Str>>] method_name:Main.doThrow
+ /// CHECK: InvokeVirtual [{{l\d+}},<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
public void doit2(int[] a) {
// Being in the boot image means we know the load string cannot throw. Create one that is
@@ -142,7 +142,7 @@
/// CHECK: If [<<Tst>>]
/// CHECK: end_block
/// CHECK: begin_block
- /// CHECK: InvokeStaticOrDirect [{{l\d+}},<<Str>>] method_name:Main.doThrow
+ /// CHECK: InvokeVirtual [{{l\d+}},<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
//
/// CHECK-START: void Main.doit4(int[]) code_sinking (after)
@@ -152,7 +152,7 @@
/// CHECK: end_block
/// CHECK: begin_block
/// CHECK: <<Str:l\d+>> LoadString
- /// CHECK: InvokeStaticOrDirect [{{l\d+}},<<Str>>] method_name:Main.doThrow
+ /// CHECK: InvokeVirtual [{{l\d+}},<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
public void doit4(int[] a) {
// Being in the boot image means we know the load string cannot throw. Create one that is
diff --git a/test/730-checker-inlining-super/expected-stderr.txt b/test/730-checker-inlining-super/expected-stderr.txt
deleted file mode 100644
index e69de29..0000000
--- a/test/730-checker-inlining-super/expected-stderr.txt
+++ /dev/null
diff --git a/test/730-checker-inlining-super/expected-stdout.txt b/test/730-checker-inlining-super/expected-stdout.txt
deleted file mode 100644
index e69de29..0000000
--- a/test/730-checker-inlining-super/expected-stdout.txt
+++ /dev/null
diff --git a/test/730-checker-inlining-super/info.txt b/test/730-checker-inlining-super/info.txt
deleted file mode 100644
index d34596a..0000000
--- a/test/730-checker-inlining-super/info.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Test that an invoke-virtual devirtualized by the compiler uses the right bss
-slot at runtime.
diff --git a/test/730-checker-inlining-super/src/Main.java b/test/730-checker-inlining-super/src/Main.java
deleted file mode 100644
index 5a1b574..0000000
--- a/test/730-checker-inlining-super/src/Main.java
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * 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.
- */
-
-class SuperClass {
- public int doInvoke() {
- synchronized (this) {
- return 42;
- }
- }
-}
-
-public class Main extends SuperClass {
-
- public static void main(String[] args) {
- Main m = new Main();
- int value = doInvokeTypedSuperClass(m);
- if (value != 43) {
- throw new Error("Expected 43, got " + value);
- }
- }
-
- public static int doInvokeTypedSuperClass(SuperClass sc) {
- return sc.doInvoke();
- }
-
- public int doInvoke() {
- synchronized (this) {
- return super.doInvoke() + 1;
- }
- }
-}
diff --git a/test/823-cha-inlining/src/Main.java b/test/823-cha-inlining/src/Main.java
index c683e6b..e25047d 100644
--- a/test/823-cha-inlining/src/Main.java
+++ b/test/823-cha-inlining/src/Main.java
@@ -92,7 +92,7 @@
$noinline$doCallDefaultConflictItf3();
throw new Error("Expected IncompatibleClassChangeError");
} catch (Exception e) {
- throw new Error("Unexpected exception " + e);
+ throw new Error("Unexpected exception");
} catch (IncompatibleClassChangeError e) {
// Expected.
}
diff --git a/test/utils/regen-test-files b/test/utils/regen-test-files
index 1d81e31..cd9de74 100755
--- a/test/utils/regen-test-files
+++ b/test/utils/regen-test-files
@@ -114,7 +114,6 @@
"177-visibly-initialized-deadlock",
"178-app-image-native-method",
"179-nonvirtual-jni",
- "450-checker-types",
"1900-track-alloc",
"1901-get-bytecodes",
"1902-suspend",