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
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index b28c2d9..fbec52d 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -113,8 +113,10 @@
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 @@
// 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 @@
// 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.
diff --git a/compiler/optimizing/builder.h b/compiler/optimizing/builder.h
index a59e782..2964ad5 100644
--- a/compiler/optimizing/builder.h
+++ b/compiler/optimizing/builder.h
@@ -56,7 +56,8 @@
// 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_;
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 44e342f..278d232 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -137,6 +137,7 @@
kAnalysisInvalidBytecode,
kAnalysisFailThrowCatchLoop,
kAnalysisFailAmbiguousArrayOp,
+ kAnalysisFailInliningIrreducibleLoop,
kAnalysisFailIrreducibleLoopAndStringInit,
kAnalysisFailPhiEquivalentInOsr,
kAnalysisSuccess,
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index b4bf477..76e3596 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -869,6 +869,11 @@
MethodCompilationStat::kNotCompiledAmbiguousArrayOp);
break;
}
+ case kAnalysisFailInliningIrreducibleLoop: {
+ MaybeRecordStat(compilation_stats_.get(),
+ MethodCompilationStat::kNotCompiledInliningIrreducibleLoop);
+ break;
+ }
case kAnalysisFailIrreducibleLoopAndStringInit: {
MaybeRecordStat(compilation_stats_.get(),
MethodCompilationStat::kNotCompiledIrreducibleLoopAndStringInit);
diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h
index 313f3ef..24a28e3 100644
--- a/compiler/optimizing/optimizing_compiler_stats.h
+++ b/compiler/optimizing/optimizing_compiler_stats.h
@@ -59,6 +59,7 @@
kNotCompiledSpaceFilter,
kNotCompiledUnhandledInstruction,
kNotCompiledUnsupportedIsa,
+ kNotCompiledInliningIrreducibleLoop,
kNotCompiledIrreducibleLoopAndStringInit,
kNotCompiledPhiEquivalentInOsr,
kInlinedMonomorphicCall,
diff --git a/test/559-checker-irreducible-loop/expected-stdout.txt b/test/559-checker-irreducible-loop/expected-stdout.txt
index b64be7a..992fd87 100644
--- a/test/559-checker-irreducible-loop/expected-stdout.txt
+++ b/test/559-checker-irreducible-loop/expected-stdout.txt
@@ -5,3 +5,5 @@
class Main
42
-42
+1
+-1
diff --git a/test/559-checker-irreducible-loop/smali/IrreducibleLoop.smali b/test/559-checker-irreducible-loop/smali/IrreducibleLoop.smali
index 493567c..a88422f 100644
--- a/test/559-checker-irreducible-loop/smali/IrreducibleLoop.smali
+++ b/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
diff --git a/test/559-checker-irreducible-loop/src/Main.java b/test/559-checker-irreducible-loop/src/Main.java
index b22e9b8..16b8415 100644
--- a/test/559-checker-irreducible-loop/src/Main.java
+++ b/test/559-checker-irreducible-loop/src/Main.java
@@ -60,6 +60,18 @@
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;