Add irreducible loop check in CanInlineBody

In aosp/2335809 we moved splitting the critical edge (aka adding
the extra goto) from the builder to InlineInto. We need to
also move the irreducible loop check from there to inliner.cc.

Bug: 262725735
Fixes: 262725735
Test: dex2oat compiling the apps in the bug
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Change-Id: I94eebfb21cd94b1199ba996d458b897b7917840e

8 files changed, 152 insertions, 8 deletions

diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 2645cf1e49..3992ff4862 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1896,11 +1896,25 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
   bool has_one_return = false;
   for (HBasicBlock* predecessor : exit_block->GetPredecessors()) {
     const HInstruction* last_instruction = predecessor->GetLastInstruction();
-    // On inlinees, we can have Throw -> TryBoundary -> Exit. To check for the actual last
-    // instruction, we have to skip it.
+    // On inlinees, we can have Return/ReturnVoid/Throw -> TryBoundary -> Exit. To check for the
+    // actual last instruction, we have to skip the TryBoundary instruction.
     if (last_instruction->IsTryBoundary()) {
       predecessor = predecessor->GetSinglePredecessor();
       last_instruction = predecessor->GetLastInstruction();
+
+      // If the last instruction chain is Return/ReturnVoid -> TryBoundary -> Exit we will have to
+      // split a critical edge in InlineInto and might recompute loop information, which is
+      // unsupported for irreducible loops.
+      if (!last_instruction->IsThrow() && graph_->HasIrreducibleLoops()) {
+        DCHECK(last_instruction->IsReturn() || last_instruction->IsReturnVoid());
+        // TODO(ngeoffray): Support re-computing loop information to graphs with
+        // irreducible loops?
+        LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedIrreducibleLoopCaller)
+            << "Method " << resolved_method->PrettyMethod()
+            << " could not be inlined because we will have to recompute the loop information and"
+            << " the caller has irreducible loops";
+        return false;
+      }
     }
 
     if (last_instruction->IsThrow()) {
@@ -1914,9 +1928,10 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
       } else if (graph_->HasIrreducibleLoops()) {
         // TODO(ngeoffray): Support re-computing loop information to graphs with
         // irreducible loops?
-        VLOG(compiler) << "Method " << resolved_method->PrettyMethod()
-                       << " could not be inlined because one branch always throws and"
-                       << " caller has irreducible loops";
+        LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedIrreducibleLoopCaller)
+            << "Method " << resolved_method->PrettyMethod()
+            << " could not be inlined because one branch always throws and"
+            << " the caller has irreducible loops";
         return false;
       }
     } else {
@@ -1952,7 +1967,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
       if (block->GetLoopInformation()->IsIrreducible()) {
         // Don't inline methods with irreducible loops, they could prevent some
         // optimizations to run.
-        LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedIrreducibleLoop)
+        LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedIrreducibleLoopCallee)
             << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because it contains an irreducible loop";
         return false;
diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h
index c741b02751..100441aaff 100644
--- a/compiler/optimizing/optimizing_compiler_stats.h
+++ b/compiler/optimizing/optimizing_compiler_stats.h
@@ -92,7 +92,8 @@ enum class MethodCompilationStat {
   kNotInlinedEnvironmentBudget,
   kNotInlinedInstructionBudget,
   kNotInlinedLoopWithoutExit,
-  kNotInlinedIrreducibleLoop,
+  kNotInlinedIrreducibleLoopCallee,
+  kNotInlinedIrreducibleLoopCaller,
   kNotInlinedAlwaysThrows,
   kNotInlinedInfiniteLoop,
   kNotInlinedTryCatchCallee,
diff --git a/test/2251-checker-irreducible-loop-do-not-inline/expected-stderr.txt b/test/2251-checker-irreducible-loop-do-not-inline/expected-stderr.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/2251-checker-irreducible-loop-do-not-inline/expected-stderr.txt
diff --git a/test/2251-checker-irreducible-loop-do-not-inline/expected-stdout.txt b/test/2251-checker-irreducible-loop-do-not-inline/expected-stdout.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/2251-checker-irreducible-loop-do-not-inline/expected-stdout.txt
diff --git a/test/2251-checker-irreducible-loop-do-not-inline/info.txt b/test/2251-checker-irreducible-loop-do-not-inline/info.txt
new file mode 100644
index 0000000000..6feb5a5cbe
--- /dev/null
+++ b/test/2251-checker-irreducible-loop-do-not-inline/info.txt
@@ -0,0 +1,3 @@
+Tests that we don't inline a callee with
+  Return -> TryBoundary ->Exit
+chain if the caller has irreducible loops.
diff --git a/test/2251-checker-irreducible-loop-do-not-inline/smali/IrreducibleLoop.smali b/test/2251-checker-irreducible-loop-do-not-inline/smali/IrreducibleLoop.smali
new file mode 100644
index 0000000000..700f73c100
--- /dev/null
+++ b/test/2251-checker-irreducible-loop-do-not-inline/smali/IrreducibleLoop.smali
@@ -0,0 +1,58 @@
+# Copyright (C) 2022 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.
+
+.class public LIrreducibleLoop;
+
+.super Ljava/lang/Object;
+
+# Back-edges in the ascii-art graphs are represented with dash '-'.
+#
+# Check that testDoNotInlineInner has a simple irreducible loop
+#
+#        entry
+#       /    \
+#      /      \
+# loop_entry   \
+#    /    \-    \
+# try_start\-    \
+#           other_loop_entry
+#
+# Consistency check: we didn't optimize away the irreducible loop
+## CHECK-START: java.lang.Object IrreducibleLoop.testDoNotInlineInner(java.lang.Object) register (after)
+## CHECK: irreducible:true
+#
+# We shouldn't inline `inner`.
+## CHECK-START: java.lang.Object IrreducibleLoop.testDoNotInlineInner(java.lang.Object) inliner (before)
+## CHECK: InvokeStaticOrDirect method_name:Main.inner
+#
+## CHECK-START: java.lang.Object IrreducibleLoop.testDoNotInlineInner(java.lang.Object) inliner (after)
+## CHECK: InvokeStaticOrDirect method_name:Main.inner
+.method public static testDoNotInlineInner(Ljava/lang/Object;)Ljava/lang/Object;
+  .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, :continue
+  add-int v0, v0, v0
+  :other_loop_entry
+  add-int v0, v0, v0
+  goto :loop_entry
+
+  :continue
+  invoke-static {p0}, LMain;->inner(Ljava/lang/Object;)Ljava/lang/Object;
+  move-result-object v0
+  return-object v0
+.end method
diff --git a/test/2251-checker-irreducible-loop-do-not-inline/src/Main.java b/test/2251-checker-irreducible-loop-do-not-inline/src/Main.java
new file mode 100644
index 0000000000..94815db59c
--- /dev/null
+++ b/test/2251-checker-irreducible-loop-do-not-inline/src/Main.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+import java.lang.reflect.Method;
+
+public class Main {
+    public static void main(String[] args) throws Exception {
+        Object[] arguments = {new Object()};
+        Object result = Class.forName("IrreducibleLoop")
+                                .getMethod("testDoNotInlineInner", Object.class)
+                                .invoke(null, arguments);
+        if (result == null) {
+            throw new Exception("Expected non-null result");
+        }
+    }
+
+    // Simple method to have a call inside of the synchronized block.
+    private static Object $noinline$call() {
+        return new Object();
+    }
+
+    // `inner` has a Return -> TryBoundary -> Exit chain, which means that when we inline it we
+    // would need to recompute the loop information.
+
+    // Consistency check: Three try boundary kind:exit. One for the explicit try catch, and two for
+    // the synchronized block (normal, and exceptional path).
+
+    /// CHECK-START: java.lang.Object Main.inner(java.lang.Object) builder (after)
+    /// CHECK:     TryBoundary kind:exit
+    /// CHECK:     TryBoundary kind:exit
+    /// CHECK:     TryBoundary kind:exit
+    /// CHECK-NOT: TryBoundary kind:exit
+
+    /// CHECK-START: java.lang.Object Main.inner(java.lang.Object) builder (after)
+    /// CHECK: Return loop:B2
+
+    /// CHECK-START: java.lang.Object Main.inner(java.lang.Object) builder (after)
+    /// CHECK: TryBoundary kind:exit loop:B2
+    /// CHECK: TryBoundary kind:exit loop:B2
+    /// CHECK: TryBoundary kind:exit loop:B2
+    public static Object inner(Object o) {
+        for (int i = 0; i < 4; i++) {
+            try {
+                synchronized (o) {
+                    return $noinline$call();
+                }
+            } catch (Error e) {
+                continue;
+            }
+        }
+        return null;
+    }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 09618e23bd..e04852a2e0 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1074,7 +1074,8 @@
           "1947-breakpoint-redefine-deopt",
           "2041-bad-cleaner",
           "2230-profile-save-hotness",
-          "2245-checker-smali-instance-of-comparison"
+          "2245-checker-smali-instance-of-comparison",
+          "2251-checker-irreducible-loop-do-not-inline"
         ],
         "variant": "jvm",
         "bug": "b/73888836",