Update MaybeAddExtraGotoBlocks to bail for irreducible loops

We may have graphs with irreducible loops at this point and
recomputing the loop information is not supported.

Bug: 253242440
Test: dex2oat compiling the apps in the bug
Change-Id: I181b4fb9d9812ba2f14cd21602cf0e5a4e1fd18e

compiler/optimizing/builder.cc

@@ -113,8 +113,10 @@ static bool NeedsExtraGotoBlock(HBasicBlock* block) {
   return !last_instruction->IsThrow();
 }
 
-void HGraphBuilder::MaybeAddExtraGotoBlocks() {
-  if (graph_->GetExitBlock() == nullptr) return;
+bool HGraphBuilder::MaybeAddExtraGotoBlocks() {
+  if (graph_->GetExitBlock() == nullptr) {
+    return true;
+  }
 
   bool added_block = false;
   for (size_t pred = 0, size = graph_->GetExitBlock()->GetPredecessors().size(); pred < size;
@@ -130,13 +132,17 @@ void HGraphBuilder::MaybeAddExtraGotoBlocks() {
   // TODO(solanes): Avoid recomputing the full dominator tree by manually updating the relevant
   // information (loop information, dominance, try catch information).
   if (added_block) {
-    DCHECK(!graph_->HasIrreducibleLoops())
-        << "Recomputing loop information in graphs with irreducible loops "
-        << "is unsupported, as it could lead to loop header changes";
+    if (graph_->HasIrreducibleLoops()) {
+      // Recomputing loop information in graphs with irreducible loops is unsupported, as it could
+      // lead to loop header changes. In this case it is safe to abort since we don't inline graphs
+      // with irreducible loops anyway.
+      return false;
+    }
     graph_->ClearLoopInformation();
     graph_->ClearDominanceInformation();
     graph_->BuildDominatorTree();
   }
+  return true;
 }
 
 GraphAnalysisResult HGraphBuilder::BuildGraph(bool build_for_inline) {
@@ -193,7 +199,9 @@ GraphAnalysisResult HGraphBuilder::BuildGraph(bool build_for_inline) {
   // 5) When inlining, we want to add a Goto block if we have Return/ReturnVoid->TryBoundary->Exit
   // since we will have Return/ReturnVoid->TryBoundary->`continue to normal execution` once inlined.
   if (build_for_inline) {
-    MaybeAddExtraGotoBlocks();
+    if (!MaybeAddExtraGotoBlocks()) {
+      return kAnalysisFailInliningIrreducibleLoop;
+    }
   }
 
   // 6) Type the graph and eliminate dead/redundant phis.

compiler/optimizing/builder.h

@@ -56,7 +56,8 @@ class HGraphBuilder : public ValueObject {
 
   // When inlining, we sometimes want to add an extra Goto block before the Exit block. This is done
   // in the building phase as we do not allow the inlining phase to add new instructions.
-  void MaybeAddExtraGotoBlocks();
+  // Returns false if the graph we are adding the extra block has irreducible loops.
+  bool MaybeAddExtraGotoBlocks();
 
   HGraph* const graph_;
   const DexFile* const dex_file_;

compiler/optimizing/nodes.h

@@ -137,6 +137,7 @@ enum GraphAnalysisResult {
   kAnalysisInvalidBytecode,
   kAnalysisFailThrowCatchLoop,
   kAnalysisFailAmbiguousArrayOp,
+  kAnalysisFailInliningIrreducibleLoop,
   kAnalysisFailIrreducibleLoopAndStringInit,
   kAnalysisFailPhiEquivalentInOsr,
   kAnalysisSuccess,

compiler/optimizing/optimizing_compiler.cc

@@ -869,6 +869,11 @@ CodeGenerator* OptimizingCompiler::TryCompile(ArenaAllocator* allocator,
                           MethodCompilationStat::kNotCompiledAmbiguousArrayOp);
           break;
         }
+        case kAnalysisFailInliningIrreducibleLoop: {
+          MaybeRecordStat(compilation_stats_.get(),
+                          MethodCompilationStat::kNotCompiledInliningIrreducibleLoop);
+          break;
+        }
         case kAnalysisFailIrreducibleLoopAndStringInit: {
           MaybeRecordStat(compilation_stats_.get(),
                           MethodCompilationStat::kNotCompiledIrreducibleLoopAndStringInit);

compiler/optimizing/optimizing_compiler_stats.h

@@ -59,6 +59,7 @@ enum class MethodCompilationStat {
   kNotCompiledSpaceFilter,
   kNotCompiledUnhandledInstruction,
   kNotCompiledUnsupportedIsa,
+  kNotCompiledInliningIrreducibleLoop,
   kNotCompiledIrreducibleLoopAndStringInit,
   kNotCompiledPhiEquivalentInOsr,
   kInlinedMonomorphicCall,

test/559-checker-irreducible-loop/expected-stdout.txt

@@ -5,3 +5,5 @@
 class Main
 42
 -42
+1
+-1

test/559-checker-irreducible-loop/smali/IrreducibleLoop.smali

@@ -546,3 +546,57 @@
   :exit
   return-void
 .end method
+
+## CHECK-START: int IrreducibleLoop.testDoNotInlineIrreducible(int) inliner (before)
+## CHECK: InvokeStaticOrDirect method_name:IrreducibleLoop.doNotInlineIrreducible
+#
+## CHECK-START: int IrreducibleLoop.testDoNotInlineIrreducible(int) inliner (after)
+## CHECK: InvokeStaticOrDirect method_name:IrreducibleLoop.doNotInlineIrreducible
+.method public static testDoNotInlineIrreducible(I)I
+  .registers 2
+  invoke-static {p0}, LIrreducibleLoop;->doNotInlineIrreducible(I)I
+  move-result v0
+  return v0
+.end method
+
+# Check that doNotInlineIrreducible has a simple irreducible loop
+#
+#        entry
+#       /    \
+#      /      \
+# loop_entry   \
+#    /    \-    \
+# try_start\-    \
+#           other_loop_entry
+#
+## CHECK-START: int IrreducibleLoop.doNotInlineIrreducible(int) register (after)
+## CHECK: irreducible:true
+#
+# Check that we didn't optimized away the try.
+## CHECK-START: int IrreducibleLoop.doNotInlineIrreducible(int) register (after)
+## CHECK: TryBoundary kind:exit
+.method public static doNotInlineIrreducible(I)I
+  .registers 3
+  const/16 v0, 42
+  const/16 v1, 21
+  # Irreducible loop
+  if-eq v1, v0, :other_loop_entry
+  :loop_entry
+  if-ne v1, v0, :try_start
+  add-int v0, v0, v0
+  :other_loop_entry
+  add-int v0, v0, v0
+  goto :loop_entry
+
+  :try_start
+  # We make this division to make sure the try doesn't get optimized out
+  div-int v0, v0, p0
+  return v0
+  :try_end
+  .catchall {:try_start .. :try_end} :catch_all
+
+  :catch_all
+  # This is only reachable if the parameter is 0
+  const/4 v0, -0x1
+  return v0
+.end method

test/559-checker-irreducible-loop/src/Main.java

@@ -60,6 +60,18 @@ public static void main(String[] args) throws Exception {
       Object[] arguments = { 42 };
       System.out.println(m.invoke(null, arguments));
     }
+
+    {
+      Method m = c.getMethod("testDoNotInlineIrreducible", int.class);
+      Object[] arguments = { 42 };
+      System.out.println(m.invoke(null, arguments));
+    }
+
+    {
+      Method m = c.getMethod("testDoNotInlineIrreducible", int.class);
+      Object[] arguments = { 0 };
+      System.out.println(m.invoke(null, arguments));
+    }
   }
 
   int myField;