From: Calin Juravle Date: Mon, 19 Oct 2015 15:19:23 +0000 (+0100) Subject: Run type propagation after inliner only when needed. X-Git-Tag: android-x86-7.1-r1~889^2~72^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=4e5dd521063beae1706410419f19c7e224db50fe;p=android-x86%2Fart.git Run type propagation after inliner only when needed. Currently we run a type propagation pass unconditionally after the inliner. This change looks at the returned value (if any) and runs a minimal type propagation only if its type has changed. Change-Id: I0dd72bd481219081e8a978d2632426afc980d73a --- diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc index 4111671a9..264c94e93 100644 --- a/compiler/optimizing/graph_visualizer.cc +++ b/compiler/optimizing/graph_visualizer.cc @@ -24,6 +24,7 @@ #include "code_generator.h" #include "dead_code_elimination.h" #include "disassembler.h" +#include "inliner.h" #include "licm.h" #include "nodes.h" #include "optimization.h" @@ -424,11 +425,6 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { return strcmp(pass_name_, name) == 0; } - bool IsReferenceTypePropagationPass() { - return strstr(pass_name_, ReferenceTypePropagation::kReferenceTypePropagationPassName) - != nullptr; - } - void PrintInstruction(HInstruction* instruction) { output_ << instruction->DebugName(); if (instruction->InputCount() > 0) { @@ -492,7 +488,8 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { } else { StartAttributeStream("loop") << "B" << info->GetHeader()->GetBlockId(); } - } else if (IsReferenceTypePropagationPass() + } else if ((IsPass(ReferenceTypePropagation::kReferenceTypePropagationPassName) + || IsPass(HInliner::kInlinerPassName)) && (instruction->GetType() == Primitive::kPrimNot)) { ReferenceTypeInfo info = instruction->IsLoadClass() ? instruction->AsLoadClass()->GetLoadedClassRTI() diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index e2aca3091..7a8ac2b3e 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -512,6 +512,9 @@ bool HInliner::TryBuildAndInline(ArtMethod* resolved_method, if ((return_replacement != nullptr) && (return_replacement->GetType() == Primitive::kPrimNot)) { + // TODO(calin): If valid, return_replacement could still be object (as a result of a merge). + // This happens because we don't implement the smallest common type and merge to Object. + // In this case, we should bound its type to the type of the invoke. if (!return_replacement->GetReferenceTypeInfo().IsValid()) { // Make sure that we have a valid type for the return. We may get an invalid one when // we inline invokes with multiple branches and create a Phi for the result. @@ -520,10 +523,19 @@ bool HInliner::TryBuildAndInline(ArtMethod* resolved_method, DCHECK(return_replacement->IsPhi()); size_t pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize(); ReferenceTypeInfo::TypeHandle return_handle = - handles_->NewHandle(resolved_method->GetReturnType(true /* resolve */, pointer_size)); + handles_->NewHandle(resolved_method->GetReturnType(true /* resolve */, pointer_size)); return_replacement->SetReferenceTypeInfo(ReferenceTypeInfo::Create( return_handle, return_handle->CannotBeAssignedFromOtherTypes() /* is_exact */)); } + + // Check if the actual return type is a subtype of the declared invoke type. + // If so, run a limited type propagation to update the types in the original graph. + ReferenceTypeInfo return_rti = return_replacement->GetReferenceTypeInfo(); + ReferenceTypeInfo invoke_rti = invoke_instruction->GetReferenceTypeInfo(); + if (!return_rti.IsEqual(invoke_rti) && invoke_rti.IsSupertypeOf(return_rti)) { + ReferenceTypePropagation rtp_fixup(graph_, handles_); + rtp_fixup.Run(return_replacement); + } } return true; diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index 5404e56a5..8459331fd 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -388,19 +388,9 @@ static void MaybeRunInliner(HGraph* graph, return; } - ArenaAllocator* arena = graph->GetArena(); - HInliner* inliner = new (arena) HInliner( + HInliner* inliner = new (graph->GetArena()) HInliner( graph, dex_compilation_unit, dex_compilation_unit, driver, handles, stats); - ReferenceTypePropagation* type_propagation = - new (arena) ReferenceTypePropagation(graph, handles, - "reference_type_propagation_after_inlining"); - - HOptimization* optimizations[] = { - inliner, - // Run another type propagation phase: inlining will open up more opportunities - // to remove checkcast/instanceof and null checks. - type_propagation, - }; + HOptimization* optimizations[] = { inliner }; RunOptimizations(optimizations, arraysize(optimizations), pass_observer); } diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc index 26a05da4c..b0b7f3fe2 100644 --- a/compiler/optimizing/reference_type_propagation.cc +++ b/compiler/optimizing/reference_type_propagation.cc @@ -99,17 +99,9 @@ ReferenceTypePropagation::ReferenceTypePropagation(HGraph* graph, } } -void ReferenceTypePropagation::Run() { - // To properly propagate type info we need to visit in the dominator-based order. - // Reverse post order guarantees a node's dominators are visited first. - // We take advantage of this order in `VisitBasicBlock`. - for (HReversePostOrderIterator it(*graph_); !it.Done(); it.Advance()) { - VisitBasicBlock(it.Current()); - } - ProcessWorklist(); - +void ReferenceTypePropagation::ValidateTypes() { + // TODO: move this to the graph checker. if (kIsDebugBuild) { - // TODO: move this to the graph checker. ScopedObjectAccess soa(Thread::Current()); for (HReversePostOrderIterator it(*graph_); !it.Done(); it.Advance()) { HBasicBlock* block = it.Current(); @@ -135,6 +127,28 @@ void ReferenceTypePropagation::Run() { } } +void ReferenceTypePropagation::Run() { + // To properly propagate type info we need to visit in the dominator-based order. + // Reverse post order guarantees a node's dominators are visited first. + // We take advantage of this order in `VisitBasicBlock`. + for (HReversePostOrderIterator it(*graph_); !it.Done(); it.Advance()) { + VisitBasicBlock(it.Current()); + } + + ProcessWorklist(); + ValidateTypes(); +} + +void ReferenceTypePropagation::Run(HInstruction* start_from) { + DCHECK(start_from != nullptr); + DCHECK_EQ(graph_, start_from->GetBlock()->GetGraph()); + DCHECK_EQ(Primitive::kPrimNot, start_from->GetType()); + + AddDependentInstructionsToWorklist(start_from); + ProcessWorklist(); + ValidateTypes(); +} + void ReferenceTypePropagation::VisitBasicBlock(HBasicBlock* block) { RTPVisitor visitor(graph_, handles_, diff --git a/compiler/optimizing/reference_type_propagation.h b/compiler/optimizing/reference_type_propagation.h index 5493601ad..139f88166 100644 --- a/compiler/optimizing/reference_type_propagation.h +++ b/compiler/optimizing/reference_type_propagation.h @@ -36,6 +36,9 @@ class ReferenceTypePropagation : public HOptimization { const char* name = kReferenceTypePropagationPassName); void Run() OVERRIDE; + // Run the type propagation and propagate information only for the + // instructions dominated by `start_from`. + void Run(HInstruction* start_from); static constexpr const char* kReferenceTypePropagationPassName = "reference_type_propagation"; @@ -56,6 +59,8 @@ class ReferenceTypePropagation : public HOptimization { ReferenceTypeInfo MergeTypes(const ReferenceTypeInfo& a, const ReferenceTypeInfo& b) SHARED_REQUIRES(Locks::mutator_lock_); + void ValidateTypes(); + StackHandleScopeCollection* handles_; ArenaVector worklist_; diff --git a/test/450-checker-types/src/Main.java b/test/450-checker-types/src/Main.java index f1885def1..6db13d69b 100644 --- a/test/450-checker-types/src/Main.java +++ b/test/450-checker-types/src/Main.java @@ -454,7 +454,7 @@ public class Main { /// CHECK: <> InvokeStaticOrDirect klass:SubclassC exact:false /// CHECK-NEXT: Return [<>] - /// CHECK-START: SubclassC Main.inlineGenerics() reference_type_propagation_after_inlining (after) + /// CHECK-START: SubclassC Main.inlineGenerics() inliner (after) /// CHECK: <> BoundType klass:SubclassC exact:false /// CHECK: Return [<>] private SubclassC inlineGenerics() { @@ -466,7 +466,7 @@ public class Main { /// CHECK: <> InvokeStaticOrDirect klass:Final exact:true /// CHECK-NEXT: Return [<>] - /// CHECK-START: Final Main.inlineGenericsFinal() reference_type_propagation_after_inlining (after) + /// CHECK-START: Final Main.inlineGenericsFinal() inliner (after) /// CHECK: <> BoundType klass:Final exact:true /// CHECK: Return [<>] private Final inlineGenericsFinal() { @@ -474,7 +474,7 @@ public class Main { return f; } - /// CHECK-START: void Main.boundOnlyOnceIfNotNull(java.lang.Object) reference_type_propagation_after_inlining (after) + /// CHECK-START: void Main.boundOnlyOnceIfNotNull(java.lang.Object) inliner (after) /// CHECK: BoundType /// CHECK-NOT: BoundType private void boundOnlyOnceIfNotNull(Object o) { @@ -483,7 +483,7 @@ public class Main { } } - /// CHECK-START: void Main.boundOnlyOnceIfInstanceOf(java.lang.Object) reference_type_propagation_after_inlining (after) + /// CHECK-START: void Main.boundOnlyOnceIfInstanceOf(java.lang.Object) inliner (after) /// CHECK: BoundType /// CHECK-NOT: BoundType private void boundOnlyOnceIfInstanceOf(Object o) { @@ -492,7 +492,7 @@ public class Main { } } - /// CHECK-START: Final Main.boundOnlyOnceCheckCast(Generic) reference_type_propagation_after_inlining (after) + /// CHECK-START: Final Main.boundOnlyOnceCheckCast(Generic) inliner (after) /// CHECK: BoundType /// CHECK-NOT: BoundType private Final boundOnlyOnceCheckCast(Generic o) { @@ -508,7 +508,7 @@ public class Main { /// CHECK: <> Phi klass:Super /// CHECK: NullCheck [<>] klass:Super - /// CHECK-START: void Main.updateNodesInTheSameBlockAsPhi(boolean) reference_type_propagation_after_inlining (after) + /// CHECK-START: void Main.updateNodesInTheSameBlockAsPhi(boolean) inliner (after) /// CHECK: <> Phi klass:SubclassA /// CHECK: NullCheck [<>] klass:SubclassA private void updateNodesInTheSameBlockAsPhi(boolean cond) { @@ -519,7 +519,7 @@ public class Main { s.$noinline$f(); } - /// CHECK-START: java.lang.String Main.checkcastPreserveNullCheck(java.lang.Object) reference_type_propagation_after_inlining (after) + /// CHECK-START: java.lang.String Main.checkcastPreserveNullCheck(java.lang.Object) inliner (after) /// CHECK: <> ParameterValue /// CHECK: <> ParameterValue /// CHECK: <> LoadClass diff --git a/test/530-checker-regression-reftype-final/smali/TestCase.smali b/test/530-checker-regression-reftype-final/smali/TestCase.smali index 8fd7bb7ed..44facfca0 100644 --- a/test/530-checker-regression-reftype-final/smali/TestCase.smali +++ b/test/530-checker-regression-reftype-final/smali/TestCase.smali @@ -23,7 +23,7 @@ # inline any methods from array classes, this bug cannot be triggered and we # verify it using Checker. -## CHECK-START: void TestCase.testInliner() reference_type_propagation_after_inlining (before) +## CHECK-START: void TestCase.testInliner() inliner (after) ## CHECK-DAG: CheckCast [<>,{{l\d+}}] ## CHECK-DAG: <> Phi klass:java.lang.Object[] exact:false