OSDN Git Service

Run type propagation after inliner only when needed.
authorCalin Juravle <calin@google.com>
Mon, 19 Oct 2015 15:19:23 +0000 (16:19 +0100)
committerCalin Juravle <calin@google.com>
Fri, 23 Oct 2015 16:18:35 +0000 (17:18 +0100)
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

compiler/optimizing/graph_visualizer.cc
compiler/optimizing/inliner.cc
compiler/optimizing/optimizing_compiler.cc
compiler/optimizing/reference_type_propagation.cc
compiler/optimizing/reference_type_propagation.h
test/450-checker-types/src/Main.java
test/530-checker-regression-reftype-final/smali/TestCase.smali

index 4111671..264c94e 100644 (file)
@@ -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()
index e2aca30..7a8ac2b 100644 (file)
@@ -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;
index 5404e56..8459331 100644 (file)
@@ -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);
 }
index 26a05da..b0b7f3f 100644 (file)
@@ -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_,
index 5493601..139f881 100644 (file)
@@ -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<HInstruction*> worklist_;
index f1885de..6db13d6 100644 (file)
@@ -454,7 +454,7 @@ public class Main {
   /// CHECK:      <<Invoke:l\d+>>    InvokeStaticOrDirect klass:SubclassC exact:false
   /// CHECK-NEXT:                    Return [<<Invoke>>]
 
-  /// CHECK-START: SubclassC Main.inlineGenerics() reference_type_propagation_after_inlining (after)
+  /// CHECK-START: SubclassC Main.inlineGenerics() inliner (after)
   /// CHECK:      <<BoundType:l\d+>> BoundType klass:SubclassC exact:false
   /// CHECK:                         Return [<<BoundType>>]
   private SubclassC inlineGenerics() {
@@ -466,7 +466,7 @@ public class Main {
   /// CHECK:      <<Invoke:l\d+>>    InvokeStaticOrDirect klass:Final exact:true
   /// CHECK-NEXT:                    Return [<<Invoke>>]
 
-  /// CHECK-START: Final Main.inlineGenericsFinal() reference_type_propagation_after_inlining (after)
+  /// CHECK-START: Final Main.inlineGenericsFinal() inliner (after)
   /// CHECK:      <<BoundType:l\d+>> BoundType klass:Final exact:true
   /// CHECK:                         Return [<<BoundType>>]
   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<Final> o) {
@@ -508,7 +508,7 @@ public class Main {
   /// CHECK:      <<Phi:l\d+>> Phi klass:Super
   /// CHECK:                   NullCheck [<<Phi>>] klass:Super
 
-  /// CHECK-START: void Main.updateNodesInTheSameBlockAsPhi(boolean) reference_type_propagation_after_inlining (after)
+  /// CHECK-START: void Main.updateNodesInTheSameBlockAsPhi(boolean) inliner (after)
   /// CHECK:      <<Phi:l\d+>> Phi klass:SubclassA
   /// CHECK:                   NullCheck [<<Phi>>] 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:      <<This:l\d+>>     ParameterValue
   /// CHECK:      <<Param:l\d+>>    ParameterValue
   /// CHECK:      <<Clazz:l\d+>>    LoadClass
index 8fd7bb7..44facfc 100644 (file)
@@ -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 [<<Phi:l\d+>>,{{l\d+}}]
 ## CHECK-DAG:    <<Phi>>  Phi klass:java.lang.Object[] exact:false