Inline methods that throw.

Forked from https://android-review.googlesource.com/214292

test: 637-checker-throw-inline
bug: 30933338
Change-Id: I184be82dfab0710af3f3796e9e486c7817fa9c60

10 files changed, 193 insertions, 72 deletions

diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 2dd5fefc80..e012a4287f 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -69,17 +69,13 @@ void HInliner::Run() {
     // doing some logic in the runtime to discover if a method could have been inlined.
     return;
   }
-  const ArenaVector<HBasicBlock*>& blocks = graph_->GetReversePostOrder();
+  // Keep a copy of all blocks when starting the visit.
+  ArenaVector<HBasicBlock*> blocks = graph_->GetReversePostOrder();
   DCHECK(!blocks.empty());
-  HBasicBlock* next_block = blocks[0];
-  for (size_t i = 0; i < blocks.size(); ++i) {
-    // Because we are changing the graph when inlining, we need to remember the next block.
-    // This avoids doing the inlining work again on the inlined blocks.
-    if (blocks[i] != next_block) {
-      continue;
-    }
-    HBasicBlock* block = next_block;
-    next_block = (i == blocks.size() - 1) ? nullptr : blocks[i + 1];
+  // Because we are changing the graph when inlining,
+  // we just iterate over the blocks of the outer method.
+  // This avoids doing the inlining work again on the inlined blocks.
+  for (HBasicBlock* block : blocks) {
     for (HInstruction* instruction = block->GetFirstInstruction(); instruction != nullptr;) {
       HInstruction* next = instruction->GetNext();
       HInvoke* call = instruction->AsInvoke();
@@ -564,7 +560,6 @@ HInstruction* HInliner::AddTypeGuard(HInstruction* receiver,
   bb_cursor->InsertInstructionAfter(load_class, receiver_class);
   load_class->SetLoadKind(kind);
 
-  // TODO: Extend reference type propagation to understand the guard.
   HNotEqual* compare = new (graph_->GetArena()) HNotEqual(load_class, receiver_class);
   bb_cursor->InsertInstructionAfter(compare, load_class);
   if (with_deoptimization) {
@@ -848,7 +843,6 @@ bool HInliner::TryInlinePolymorphicCallToSameTarget(
   if (outermost_graph_->IsCompilingOsr()) {
     CreateDiamondPatternForPolymorphicInline(compare, return_replacement, invoke_instruction);
   } else {
-    // TODO: Extend reference type propagation to understand the guard.
     HDeoptimize* deoptimize = new (graph_->GetArena()) HDeoptimize(
         compare, invoke_instruction->GetDexPc());
     bb_cursor->InsertInstructionAfter(deoptimize, compare);
@@ -1354,9 +1348,6 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction,
       RunOptimizations(callee_graph, code_item, dex_compilation_unit);
   number_of_instructions_budget += number_of_inlined_instructions;
 
-  // TODO: We should abort only if all predecessors throw. However,
-  // HGraph::InlineInto currently does not handle an exit block with
-  // a throw predecessor.
   HBasicBlock* exit_block = callee_graph->GetExitBlock();
   if (exit_block == nullptr) {
     VLOG(compiler) << "Method " << callee_dex_file.PrettyMethod(method_index)
@@ -1364,16 +1355,30 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction,
     return false;
   }
 
-  bool has_throw_predecessor = false;
+  bool has_one_return = false;
   for (HBasicBlock* predecessor : exit_block->GetPredecessors()) {
     if (predecessor->GetLastInstruction()->IsThrow()) {
-      has_throw_predecessor = true;
-      break;
+      if (invoke_instruction->GetBlock()->IsTryBlock()) {
+        // TODO(ngeoffray): Support adding HTryBoundary in Hgraph::InlineInto.
+        VLOG(compiler) << "Method " << callee_dex_file.PrettyMethod(method_index)
+                       << " 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.
+        VLOG(compiler) << "Method " << callee_dex_file.PrettyMethod(method_index)
+                       << " could not be inlined because one branch always throws and"
+                       << " caller does not have an exit block";
+        return false;
+      }
+    } else {
+      has_one_return = true;
     }
   }
-  if (has_throw_predecessor) {
+
+  if (!has_one_return) {
     VLOG(compiler) << "Method " << callee_dex_file.PrettyMethod(method_index)
-                   << " could not be inlined because one branch always throws";
+                   << " could not be inlined because it always throws";
     return false;
   }
 
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index abbb91a1a9..71a26ebe79 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -2038,6 +2038,8 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) {
 
   HInstruction* return_value = nullptr;
   if (GetBlocks().size() == 3) {
+    // Inliner already made sure we don't inline methods that always throw.
+    DCHECK(!GetBlocks()[1]->GetLastInstruction()->IsThrow());
     // Simple case of an entry block, a body block, and an exit block.
     // Put the body block's instruction into `invoke`'s block.
     HBasicBlock* body = GetBlocks()[1];
@@ -2119,33 +2121,60 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) {
     UpdateLoopAndTryInformationOfNewBlock(to, at, /* replace_if_back_edge */ true);
 
     // Update all predecessors of the exit block (now the `to` block)
-    // to not `HReturn` but `HGoto` instead.
-    bool returns_void = to->GetPredecessors()[0]->GetLastInstruction()->IsReturnVoid();
-    if (to->GetPredecessors().size() == 1) {
-      HBasicBlock* predecessor = to->GetPredecessors()[0];
+    // to not `HReturn` but `HGoto` instead. Special case throwing blocks
+    // to now get the outer graph exit block as successor. Note that the inliner
+    // currently doesn't support inlining methods with try/catch.
+    HPhi* return_value_phi = nullptr;
+    bool rerun_dominance = false;
+    bool rerun_loop_analysis = false;
+    for (size_t pred = 0; pred < to->GetPredecessors().size(); ++pred) {
+      HBasicBlock* predecessor = to->GetPredecessors()[pred];
       HInstruction* last = predecessor->GetLastInstruction();
-      if (!returns_void) {
-        return_value = last->InputAt(0);
-      }
-      predecessor->AddInstruction(new (allocator) HGoto(last->GetDexPc()));
-      predecessor->RemoveInstruction(last);
-    } else {
-      if (!returns_void) {
-        // There will be multiple returns.
-        return_value = new (allocator) HPhi(
-            allocator, kNoRegNumber, 0, HPhi::ToPhiType(invoke->GetType()), to->GetDexPc());
-        to->AddPhi(return_value->AsPhi());
-      }
-      for (HBasicBlock* predecessor : to->GetPredecessors()) {
-        HInstruction* last = predecessor->GetLastInstruction();
-        if (!returns_void) {
+      if (last->IsThrow()) {
+        DCHECK(!at->IsTryBlock());
+        predecessor->ReplaceSuccessor(to, outer_graph->GetExitBlock());
+        --pred;
+        // We need to re-run dominance information, as the exit block now has
+        // a new dominator.
+        rerun_dominance = true;
+        if (predecessor->GetLoopInformation() != nullptr) {
+          // The exit block and blocks post dominated by the exit block do not belong
+          // to any loop. Because we do not compute the post dominators, we need to re-run
+          // loop analysis to get the loop information correct.
+          rerun_loop_analysis = true;
+        }
+      } else {
+        if (last->IsReturnVoid()) {
+          DCHECK(return_value == nullptr);
+          DCHECK(return_value_phi == nullptr);
+        } else {
           DCHECK(last->IsReturn());
-          return_value->AsPhi()->AddInput(last->InputAt(0));
+          if (return_value_phi != nullptr) {
+            return_value_phi->AddInput(last->InputAt(0));
+          } else if (return_value == nullptr) {
+            return_value = last->InputAt(0);
+          } else {
+            // There will be multiple returns.
+            return_value_phi = new (allocator) HPhi(
+                allocator, kNoRegNumber, 0, HPhi::ToPhiType(invoke->GetType()), to->GetDexPc());
+            to->AddPhi(return_value_phi);
+            return_value_phi->AddInput(return_value);
+            return_value_phi->AddInput(last->InputAt(0));
+            return_value = return_value_phi;
+          }
         }
         predecessor->AddInstruction(new (allocator) HGoto(last->GetDexPc()));
         predecessor->RemoveInstruction(last);
       }
     }
+    if (rerun_loop_analysis) {
+      outer_graph->ClearLoopInformation();
+      outer_graph->ClearDominanceInformation();
+      outer_graph->BuildDominatorTree();
+    } else if (rerun_dominance) {
+      outer_graph->ClearDominanceInformation();
+      outer_graph->ComputeDominanceInformation();
+    }
   }
 
   // Walk over the entry block and:
diff --git a/runtime/jit/profiling_info.cc b/runtime/jit/profiling_info.cc
index 405280dd47..7d80d2c208 100644
--- a/runtime/jit/profiling_info.cc
+++ b/runtime/jit/profiling_info.cc
@@ -77,20 +77,18 @@ bool ProfilingInfo::Create(Thread* self, ArtMethod* method, bool retry_allocatio
 }
 
 InlineCache* ProfilingInfo::GetInlineCache(uint32_t dex_pc) {
-  InlineCache* cache = nullptr;
   // TODO: binary search if array is too long.
   for (size_t i = 0; i < number_of_inline_caches_; ++i) {
     if (cache_[i].dex_pc_ == dex_pc) {
-      cache = &cache_[i];
-      break;
+      return &cache_[i];
     }
   }
-  return cache;
+  LOG(FATAL) << "No inline cache found for "  << ArtMethod::PrettyMethod(method_) << "@" << dex_pc;
+  UNREACHABLE();
 }
 
 void ProfilingInfo::AddInvokeInfo(uint32_t dex_pc, mirror::Class* cls) {
   InlineCache* cache = GetInlineCache(dex_pc);
-  CHECK(cache != nullptr) << ArtMethod::PrettyMethod(method_) << "@" << dex_pc;
   for (size_t i = 0; i < InlineCache::kIndividualCacheSize; ++i) {
     mirror::Class* existing = cache->classes_[i].Read();
     if (existing == cls) {
diff --git a/runtime/jit/profiling_info.h b/runtime/jit/profiling_info.h
index 9fbf2e3afa..1c58a83679 100644
--- a/runtime/jit/profiling_info.h
+++ b/runtime/jit/profiling_info.h
@@ -73,7 +73,9 @@ class ProfilingInfo {
     return method_;
   }
 
-  InlineCache* GetInlineCache(uint32_t dex_pc);
+  // Mutator lock only required for debugging output.
+  InlineCache* GetInlineCache(uint32_t dex_pc)
+      REQUIRES_SHARED(Locks::mutator_lock_);
 
   bool IsMethodBeingCompiled(bool osr) const {
     return osr
diff --git a/test/476-checker-ctor-memory-barrier/src/Main.java b/test/476-checker-ctor-memory-barrier/src/Main.java
index c2a2a100fb..65486e9e41 100644
--- a/test/476-checker-ctor-memory-barrier/src/Main.java
+++ b/test/476-checker-ctor-memory-barrier/src/Main.java
@@ -27,15 +27,10 @@ class ClassWithFinals {
   public ClassWithFinals obj;
   public static boolean doThrow = false;
 
-  /// CHECK-START: void ClassWithFinals.<init>(boolean) register (after)
-  /// CHECK:      MemoryBarrier kind:StoreStore
-  /// CHECK-NEXT: ReturnVoid
   public ClassWithFinals(boolean cond) {
-    x = 0;
-    if (doThrow) {
-      // avoid inlining
-      throw new RuntimeException();
-    }
+    x = 1;
+    throw new RuntimeException();
+    // should not inline this constructor
   }
 
   /// CHECK-START: void ClassWithFinals.<init>() register (after)
@@ -146,6 +141,7 @@ public class Main {
   /// CHECK-NOT:  MemoryBarrier kind:StoreStore
   public static ClassWithFinals noInlineNoConstructorBarrier() {
     return new ClassWithFinals(false);
+    // should not inline the constructor
   }
 
   /// CHECK-START: void Main.inlineNew() register (after)
diff --git a/test/478-checker-clinit-check-pruning/src/Main.java b/test/478-checker-clinit-check-pruning/src/Main.java
index c2982b48db..63e2b9503c 100644
--- a/test/478-checker-clinit-check-pruning/src/Main.java
+++ b/test/478-checker-clinit-check-pruning/src/Main.java
@@ -400,8 +400,10 @@ public class Main {
   /// CHECK-NOT:                           ClinitCheck
 
   static void inlinedInvokeStaticViaNonStatic(Iterable<?> it) {
-    inlinedInvokeStaticViaNonStaticHelper(null);
-    inlinedInvokeStaticViaNonStaticHelper(it);
+    if (it != null) {
+      inlinedInvokeStaticViaNonStaticHelper(null);
+      inlinedInvokeStaticViaNonStaticHelper(it);
+    }
   }
 
   static void inlinedInvokeStaticViaNonStaticHelper(Iterable<?> it) {
@@ -417,8 +419,8 @@ public class Main {
     static void inlinedForNull(Iterable<?> it) {
       if (it != null) {
         it.iterator();
-        // We're not inlining throw at the moment.
-        if (doThrow) { throw new Error(""); }
+        // We're not inlining methods that always throw.
+        throw new Error("");
       }
     }
   }
@@ -441,7 +443,9 @@ public class Main {
   /// CHECK-NOT:                           ClinitCheck
 
   static void inlinedInvokeStaticViaStatic(Iterable<?> it) {
-    ClassWithClinit11.callInlinedForNull(it);
+    if (it != null) {
+      ClassWithClinit11.callInlinedForNull(it);
+    }
   }
 
   static class ClassWithClinit11 {
@@ -457,8 +461,8 @@ public class Main {
     static void inlinedForNull(Iterable<?> it) {
       it.iterator();
       if (it != null) {
-        // We're not inlining throw at the moment.
-        if (doThrow) { throw new Error(""); }
+        // We're not inlining methods that always throw.
+        throw new Error("");
       }
     }
   }
@@ -476,8 +480,10 @@ public class Main {
   /// CHECK-NOT:                           ClinitCheck
 
   static void inlinedInvokeStaticViaStaticTwice(Iterable<?> it) {
-    ClassWithClinit12.callInlinedForNull(null);
-    ClassWithClinit12.callInlinedForNull(it);
+    if (it != null) {
+      ClassWithClinit12.callInlinedForNull(null);
+      ClassWithClinit12.callInlinedForNull(it);
+    }
   }
 
   static class ClassWithClinit12 {
@@ -492,8 +498,8 @@ public class Main {
 
     static void inlinedForNull(Iterable<?> it) {
       if (it != null) {
-        // We're not inlining throw at the moment.
-        if (doThrow) { throw new Error(""); }
+        // We're not inlining methods that always throw.
+        throw new Error("");
       }
     }
   }
@@ -537,9 +543,21 @@ public class Main {
     constClassAndInvokeStatic(it);
     sgetAndInvokeStatic(it);
     constClassSgetAndInvokeStatic(it);
-    inlinedInvokeStaticViaNonStatic(it);
-    inlinedInvokeStaticViaStatic(it);
-    inlinedInvokeStaticViaStaticTwice(it);
+    try {
+      inlinedInvokeStaticViaNonStatic(it);
+    } catch (Error e) {
+      // Expected
+    }
+    try {
+      inlinedInvokeStaticViaStatic(it);
+    } catch (Error e) {
+      // Expected
+    }
+    try{
+      inlinedInvokeStaticViaStaticTwice(it);
+    } catch (Error e) {
+      // Expected
+    }
     $noinline$testInliningAndNewInstance(it);
   }
 }
diff --git a/test/609-checker-inline-interface/src/Main.java b/test/609-checker-inline-interface/src/Main.java
index 413f2dd51d..249b7781f0 100644
--- a/test/609-checker-inline-interface/src/Main.java
+++ b/test/609-checker-inline-interface/src/Main.java
@@ -21,12 +21,21 @@ public final class Main implements Interface {
   }
 
   public void doCall() {
-    if (doThrow) throw new Error("");
+    // We do not inline methods that always throw.
+    throw new Error("");
   }
 
   public static void main(String[] args) {
-    testInlineInterfaceCall();
-    testInterfaceToVirtualCall();
+    try {
+      testInlineInterfaceCall();
+    } catch (Error e) {
+      // Expected
+    }
+    try {
+      testInterfaceToVirtualCall();
+    } catch (Error e) {
+      // Expected.
+    }
   }
 
   /// CHECK-START: void Main.testInlineInterfaceCall() inliner (before)
@@ -62,7 +71,6 @@ public final class Main implements Interface {
 
   static Interface itf = new Main();
   static Main m = new Main();
-  static boolean doThrow = false;
 }
 
 interface Interface {
diff --git a/test/637-checker-throw-inline/expected.txt b/test/637-checker-throw-inline/expected.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/637-checker-throw-inline/expected.txt
diff --git a/test/637-checker-throw-inline/info.txt b/test/637-checker-throw-inline/info.txt
new file mode 100644
index 0000000000..4fcf6a9b8d
--- /dev/null
+++ b/test/637-checker-throw-inline/info.txt
@@ -0,0 +1 @@
+Test that the compiler can inline methods that throw.
diff --git a/test/637-checker-throw-inline/src/Main.java b/test/637-checker-throw-inline/src/Main.java
new file mode 100644
index 0000000000..d4fbdf5e84
--- /dev/null
+++ b/test/637-checker-throw-inline/src/Main.java
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+public class Main {
+
+  public static void $inline$doCall() {
+    if (doThrow) throw new Error("");
+  }
+
+  public static void tryInline() {
+    if (doThrow) throw new Error("");
+  }
+
+  /// CHECK-START: void Main.test() inliner (before)
+  /// CHECK:      InvokeStaticOrDirect method_name:Main.$inline$doCall loop:none
+
+  /// CHECK-START: void Main.test() inliner (after)
+  /// CHECK-NOT:  InvokeStaticOrDirect method_name:Main.$inline$doCall
+  public static void test() {
+    $inline$doCall();
+  }
+
+  /// CHECK-START: void Main.testInLoop() inliner (before)
+  /// CHECK:      InvokeStaticOrDirect method_name:Main.$inline$doCall loop:{{B\d+}}
+
+  /// CHECK-START: void Main.testInLoop() inliner (after)
+  /// CHECK-NOT:  InvokeStaticOrDirect method_name:Main.$inline$doCall
+  public static void testInLoop() {
+    for (int i = 0; i < 10; ++i) {
+      $inline$doCall();
+    }
+  }
+
+  /// CHECK-START: void Main.testInInfiniteLoop() inliner (before)
+  /// CHECK:      InvokeStaticOrDirect method_name:Main.tryInline loop:{{B\d+}}
+
+  /// CHECK-START: void Main.testInInfiniteLoop() inliner (after)
+  /// CHECK:      InvokeStaticOrDirect method_name:Main.tryInline loop:{{B\d+}}
+  public static void testInInfiniteLoop() {
+    while (true) {
+      tryInline();
+    }
+  }
+
+  public static void main(String[] args) {
+    test();
+    testInLoop();
+  }
+
+  static boolean doThrow = false;
+}