From 454a481d116ec4e6dc36fab23a073017b1436d7f Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Tue, 9 Jun 2015 10:37:32 +0100 Subject: [PATCH] Revert "Revert "Support for inlining virtual and interface calls."" It now works thanks to: - https://android-review.googlesource.com/#/c/154016/ where the invoke type is changed. - The new FindMethodIndexIn method in this change, that locates the right method index relative to the caller's dex file. This reverts commit 6e4758615308bb525b6350c30468e33a2e1f2274. Change-Id: Iddba11664a9241e210fec211cd2aed9f4b90d118 --- compiler/optimizing/inliner.cc | 112 ++++++++++++++++++++- compiler/optimizing/inliner.h | 5 +- compiler/optimizing/optimizing_compiler.cc | 12 ++- test/444-checker-nce/src/Main.java | 16 +-- .../src/Main.java | 6 +- test/490-checker-inline/expected.txt | 0 test/490-checker-inline/info.txt | 1 + test/490-checker-inline/src/Main.java | 52 ++++++++++ .../expected.txt | 5 + test/493-checker-inline-invoke-interface/info.txt | 2 + .../src/Main.java | 48 +++++++++ 11 files changed, 243 insertions(+), 16 deletions(-) create mode 100644 test/490-checker-inline/expected.txt create mode 100644 test/490-checker-inline/info.txt create mode 100644 test/490-checker-inline/src/Main.java create mode 100644 test/493-checker-inline-invoke-interface/expected.txt create mode 100644 test/493-checker-inline-invoke-interface/info.txt create mode 100644 test/493-checker-inline-invoke-interface/src/Main.java diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index c3fc33735..5aeaad23c 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -27,6 +27,7 @@ #include "mirror/class_loader.h" #include "mirror/dex_cache.h" #include "nodes.h" +#include "reference_type_propagation.h" #include "register_allocator.h" #include "ssa_phi_elimination.h" #include "scoped_thread_state_change.h" @@ -57,7 +58,7 @@ void HInliner::Run() { next_block = (i == blocks.Size() - 1) ? nullptr : blocks.Get(i + 1); for (HInstruction* instruction = block->GetFirstInstruction(); instruction != nullptr;) { HInstruction* next = instruction->GetNext(); - HInvokeStaticOrDirect* call = instruction->AsInvokeStaticOrDirect(); + HInvoke* call = instruction->AsInvoke(); // As long as the call is not intrinsified, it is worth trying to inline. if (call != nullptr && call->GetIntrinsic() == Intrinsics::kNone) { // We use the original invoke type to ensure the resolution of the called method @@ -83,6 +84,93 @@ void HInliner::Run() { } } +static bool IsMethodOrDeclaringClassFinal(ArtMethod* method) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + return method->IsFinal() || method->GetDeclaringClass()->IsFinal(); +} + +/** + * Given the `resolved_method` looked up in the dex cache, try to find + * the actual runtime target of an interface or virtual call. + * Return nullptr if the runtime target cannot be proven. + */ +static ArtMethod* FindVirtualOrInterfaceTarget(HInvoke* invoke, ArtMethod* resolved_method) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + if (IsMethodOrDeclaringClassFinal(resolved_method)) { + // No need to lookup further, the resolved method will be the target. + return resolved_method; + } + + HInstruction* receiver = invoke->InputAt(0); + if (receiver->IsNullCheck()) { + // Due to multiple levels of inlining within the same pass, it might be that + // null check does not have the reference type of the actual receiver. + receiver = receiver->InputAt(0); + } + ReferenceTypeInfo info = receiver->GetReferenceTypeInfo(); + if (info.IsTop()) { + // We have no information on the receiver. + return nullptr; + } else if (!info.IsExact()) { + // We currently only support inlining with known receivers. + // TODO: Remove this check, we should be able to inline final methods + // on unknown receivers. + return nullptr; + } else if (info.GetTypeHandle()->IsInterface()) { + // Statically knowing that the receiver has an interface type cannot + // help us find what is the target method. + return nullptr; + } else if (!resolved_method->GetDeclaringClass()->IsAssignableFrom(info.GetTypeHandle().Get())) { + // The method that we're trying to call is not in the receiver's class or super classes. + return nullptr; + } + + ClassLinker* cl = Runtime::Current()->GetClassLinker(); + size_t pointer_size = cl->GetImagePointerSize(); + if (invoke->IsInvokeInterface()) { + resolved_method = info.GetTypeHandle()->FindVirtualMethodForInterface( + resolved_method, pointer_size); + } else { + DCHECK(invoke->IsInvokeVirtual()); + resolved_method = info.GetTypeHandle()->FindVirtualMethodForVirtual( + resolved_method, pointer_size); + } + + if (resolved_method == nullptr) { + // The information we had on the receiver was not enough to find + // the target method. Since we check above the exact type of the receiver, + // the only reason this can happen is an IncompatibleClassChangeError. + return nullptr; + } else if (resolved_method->IsAbstract()) { + // The information we had on the receiver was not enough to find + // the target method. Since we check above the exact type of the receiver, + // the only reason this can happen is an IncompatibleClassChangeError. + return nullptr; + } else if (IsMethodOrDeclaringClassFinal(resolved_method)) { + // A final method has to be the target method. + return resolved_method; + } else if (info.IsExact()) { + // If we found a method and the receiver's concrete type is statically + // known, we know for sure the target. + return resolved_method; + } else { + // Even if we did find a method, the receiver type was not enough to + // statically find the runtime target. + return nullptr; + } +} + +static uint32_t FindMethodIndexIn(ArtMethod* method, + const DexFile& dex_file, + uint32_t referrer_index) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + if (method->GetDexFile()->GetLocation().compare(dex_file.GetLocation()) == 0) { + return method->GetDexMethodIndex(); + } else { + return method->FindDexMethodIndexInOtherDexFile(dex_file, referrer_index); + } +} + bool HInliner::TryInline(HInvoke* invoke_instruction, uint32_t method_index) const { ScopedObjectAccess soa(Thread::Current()); const DexFile& caller_dex_file = *caller_compilation_unit_.GetDexFile(); @@ -99,6 +187,25 @@ bool HInliner::TryInline(HInvoke* invoke_instruction, uint32_t method_index) con return false; } + if (!invoke_instruction->IsInvokeStaticOrDirect()) { + resolved_method = FindVirtualOrInterfaceTarget(invoke_instruction, resolved_method); + if (resolved_method == nullptr) { + VLOG(compiler) << "Interface or virtual call to " + << PrettyMethod(method_index, caller_dex_file) + << " could not be statically determined"; + return false; + } + // We have found a method, but we need to find where that method is for the caller's + // dex file. + method_index = FindMethodIndexIn(resolved_method, caller_dex_file, method_index); + if (method_index == DexFile::kDexNoIndex) { + VLOG(compiler) << "Interface or virtual call to " + << PrettyMethod(resolved_method) + << " cannot be inlined because unaccessible to caller"; + return false; + } + } + bool same_dex_file = true; const DexFile& outer_dex_file = *outer_compilation_unit_.GetDexFile(); if (resolved_method->GetDexFile()->GetLocation().compare(outer_dex_file.GetLocation()) != 0) { @@ -247,11 +354,13 @@ bool HInliner::TryBuildAndInline(ArtMethod* resolved_method, // Run simple optimizations on the graph. HDeadCodeElimination dce(callee_graph, stats_); HConstantFolding fold(callee_graph); + ReferenceTypePropagation type_propagation(callee_graph, handles_); InstructionSimplifier simplify(callee_graph, stats_); HOptimization* optimizations[] = { &dce, &fold, + &type_propagation, &simplify, }; @@ -265,6 +374,7 @@ bool HInliner::TryBuildAndInline(ArtMethod* resolved_method, outer_compilation_unit_, dex_compilation_unit, compiler_driver_, + handles_, stats_, depth_ + 1); inliner.Run(); diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h index f7d8cf871..7465278f8 100644 --- a/compiler/optimizing/inliner.h +++ b/compiler/optimizing/inliner.h @@ -34,13 +34,15 @@ class HInliner : public HOptimization { const DexCompilationUnit& outer_compilation_unit, const DexCompilationUnit& caller_compilation_unit, CompilerDriver* compiler_driver, + StackHandleScopeCollection* handles, OptimizingCompilerStats* stats, size_t depth = 0) : HOptimization(outer_graph, true, kInlinerPassName, stats), outer_compilation_unit_(outer_compilation_unit), caller_compilation_unit_(caller_compilation_unit), compiler_driver_(compiler_driver), - depth_(depth) {} + depth_(depth), + handles_(handles) {} void Run() OVERRIDE; @@ -57,6 +59,7 @@ class HInliner : public HOptimization { const DexCompilationUnit& caller_compilation_unit_; CompilerDriver* const compiler_driver_; const size_t depth_; + StackHandleScopeCollection* const handles_; DISALLOW_COPY_AND_ASSIGN(HInliner); }; diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index f6ef2f7e8..bf0b9fac0 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -326,7 +326,7 @@ static void RunOptimizations(HGraph* graph, InstructionSimplifier simplify1(graph, stats); HBooleanSimplifier boolean_simplify(graph); - HInliner inliner(graph, dex_compilation_unit, dex_compilation_unit, driver, stats); + HInliner inliner(graph, dex_compilation_unit, dex_compilation_unit, driver, handles, stats); HConstantFolding fold2(graph, "constant_folding_after_inlining"); SideEffectsAnalysis side_effects(graph); @@ -335,6 +335,8 @@ static void RunOptimizations(HGraph* graph, BoundsCheckElimination bce(graph); ReferenceTypePropagation type_propagation(graph, handles); InstructionSimplifier simplify2(graph, stats, "instruction_simplifier_after_types"); + InstructionSimplifier simplify3(graph, stats, "last_instruction_simplifier"); + ReferenceTypePropagation type_propagation2(graph, handles); IntrinsicsRecognizer intrinsics(graph, driver); @@ -343,7 +345,12 @@ static void RunOptimizations(HGraph* graph, &dce1, &fold1, &simplify1, + &type_propagation, + &simplify2, &inliner, + // Run another type propagation phase: inlining will open up more opprotunities + // to remove checkast/instanceof and null checks. + &type_propagation2, // BooleanSimplifier depends on the InstructionSimplifier removing redundant // suspend checks to recognize empty blocks. &boolean_simplify, @@ -352,8 +359,7 @@ static void RunOptimizations(HGraph* graph, &gvn, &licm, &bce, - &type_propagation, - &simplify2, + &simplify3, &dce2, }; diff --git a/test/444-checker-nce/src/Main.java b/test/444-checker-nce/src/Main.java index 6ac0cad7e..32122e4dc 100644 --- a/test/444-checker-nce/src/Main.java +++ b/test/444-checker-nce/src/Main.java @@ -27,37 +27,37 @@ public class Main { return m.g(); } - /// CHECK-START: Main Main.thisTest() instruction_simplifier (before) + /// CHECK-START: Main Main.thisTest() ssa_builder (after) /// CHECK: NullCheck /// CHECK: InvokeStaticOrDirect - /// CHECK-START: Main Main.thisTest() instruction_simplifier (after) + /// CHECK-START: Main Main.thisTest() instruction_simplifier_after_types (after) /// CHECK-NOT: NullCheck /// CHECK: InvokeStaticOrDirect public Main thisTest() { return g(); } - /// CHECK-START: Main Main.newInstanceRemoveTest() instruction_simplifier (before) + /// CHECK-START: Main Main.newInstanceRemoveTest() ssa_builder (after) /// CHECK: NewInstance /// CHECK: NullCheck /// CHECK: InvokeStaticOrDirect /// CHECK: NullCheck /// CHECK: InvokeStaticOrDirect - /// CHECK-START: Main Main.newInstanceRemoveTest() instruction_simplifier (after) + /// CHECK-START: Main Main.newInstanceRemoveTest() instruction_simplifier_after_types (after) /// CHECK-NOT: NullCheck public Main newInstanceRemoveTest() { Main m = new Main(); return m.g(); } - /// CHECK-START: Main Main.newArrayRemoveTest() instruction_simplifier (before) + /// CHECK-START: Main Main.newArrayRemoveTest() ssa_builder (after) /// CHECK: NewArray /// CHECK: NullCheck /// CHECK: ArrayGet - /// CHECK-START: Main Main.newArrayRemoveTest() instruction_simplifier (after) + /// CHECK-START: Main Main.newArrayRemoveTest() instruction_simplifier_after_types (after) /// CHECK: NewArray /// CHECK-NOT: NullCheck /// CHECK: ArrayGet @@ -178,10 +178,10 @@ public class Main { return n.g(); } - /// CHECK-START: Main Main.scopeRemoveTest(int, Main) instruction_simplifier (before) + /// CHECK-START: Main Main.scopeRemoveTest(int, Main) ssa_builder (after) /// CHECK: NullCheck - /// CHECK-START: Main Main.scopeRemoveTest(int, Main) instruction_simplifier (after) + /// CHECK-START: Main Main.scopeRemoveTest(int, Main) instruction_simplifier_after_types (after) /// CHECK-NOT: NullCheck public Main scopeRemoveTest(int count, Main a) { Main m = null; diff --git a/test/458-checker-instruction-simplification/src/Main.java b/test/458-checker-instruction-simplification/src/Main.java index ad5fc8ef9..ef18f64a3 100644 --- a/test/458-checker-instruction-simplification/src/Main.java +++ b/test/458-checker-instruction-simplification/src/Main.java @@ -933,18 +933,18 @@ public class Main { * remove the second. */ - /// CHECK-START: boolean Main.NotNotBool(boolean) instruction_simplifier_after_types (before) + /// CHECK-START: boolean Main.NotNotBool(boolean) last_instruction_simplifier (before) /// CHECK-DAG: <> ParameterValue /// CHECK-DAG: <> BooleanNot [<>] /// CHECK-DAG: <> BooleanNot [<>] /// CHECK-DAG: Return [<>] - /// CHECK-START: boolean Main.NotNotBool(boolean) instruction_simplifier_after_types (after) + /// CHECK-START: boolean Main.NotNotBool(boolean) last_instruction_simplifier (after) /// CHECK-DAG: <> ParameterValue /// CHECK-DAG: BooleanNot [<>] /// CHECK-DAG: Return [<>] - /// CHECK-START: boolean Main.NotNotBool(boolean) instruction_simplifier_after_types (after) + /// CHECK-START: boolean Main.NotNotBool(boolean) last_instruction_simplifier (after) /// CHECK: BooleanNot /// CHECK-NOT: BooleanNot diff --git a/test/490-checker-inline/expected.txt b/test/490-checker-inline/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/490-checker-inline/info.txt b/test/490-checker-inline/info.txt new file mode 100644 index 000000000..0e42d771f --- /dev/null +++ b/test/490-checker-inline/info.txt @@ -0,0 +1 @@ +Check that we inline virtual and interface calls. diff --git a/test/490-checker-inline/src/Main.java b/test/490-checker-inline/src/Main.java new file mode 100644 index 000000000..21a01897e --- /dev/null +++ b/test/490-checker-inline/src/Main.java @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2015 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 { + public void invokeInterface(); +} + +public class Main implements Itf { + + public void invokeInterface () { + } + + public void invokeVirtual() { + } + + public static Main createMain() { + return new Main(); + } + + public static Itf createItf() { + return new Main(); + } + + /// CHECK-START: void Main.testMethod() inliner (before) + /// CHECK-DAG: InvokeVirtual + /// CHECK-DAG: InvokeInterface + + /// CHECK-START: void Main.testMethod() inliner (after) + /// CHECK-NOT: Invoke{{.*}} + + public static void testMethod() { + createMain().invokeVirtual(); + createItf().invokeInterface(); + } + + public static void main(String[] args) { + testMethod(); + } +} diff --git a/test/493-checker-inline-invoke-interface/expected.txt b/test/493-checker-inline-invoke-interface/expected.txt new file mode 100644 index 000000000..93620a6fb --- /dev/null +++ b/test/493-checker-inline-invoke-interface/expected.txt @@ -0,0 +1,5 @@ +Hello from clinit +java.lang.Exception + at ForceStatic.(Main.java:24) + at Main.foo(Main.java:31) + at Main.main(Main.java:42) diff --git a/test/493-checker-inline-invoke-interface/info.txt b/test/493-checker-inline-invoke-interface/info.txt new file mode 100644 index 000000000..bac9c82c9 --- /dev/null +++ b/test/493-checker-inline-invoke-interface/info.txt @@ -0,0 +1,2 @@ +Check that we can optimize interface calls without +requiring the verifier to sharpen them. diff --git a/test/493-checker-inline-invoke-interface/src/Main.java b/test/493-checker-inline-invoke-interface/src/Main.java new file mode 100644 index 000000000..44b727fe5 --- /dev/null +++ b/test/493-checker-inline-invoke-interface/src/Main.java @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2015 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 { + public void foo(); +} + +class ForceStatic { + static { + System.out.println("Hello from clinit"); + new Exception().printStackTrace(); + } + static int field; +} + +public class Main implements Itf { + public void foo() { + int a = ForceStatic.field; + } + + /// CHECK-START: void Main.main(java.lang.String[]) inliner (before) + /// CHECK: InvokeStaticOrDirect + /// CHECK: InvokeInterface + + /// CHECK-START: void Main.main(java.lang.String[]) inliner (after) + /// CHECK-NOT: Invoke{{.*}} + public static void main(String[] args) { + Itf itf = bar(); + itf.foo(); + } + + public static Itf bar() { + return new Main(); + } +} -- 2.11.0