Fix LoopElimination mistakenly eliminating a loop

This resulted in a crash for debug builds, and mistakes in the
optimizations for release builds.

It requires a variable to be used in the loop we are unrolling
as well as in a catch phi that it is not part of the loop we
are unrolling.

We could potentially fully unroll that loop, but this requires
a refactor on how SuperblockCloner works. This CL makes it so
that we won't optimize such loops, which should be rare enough
to not cause any visible regressions.

Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Bug: 368984521
Fixes: 368984521
Change-Id: I39f88586358573bbe76d9369d4ec67a942fd3eec

1 files changed, 18 insertions, 16 deletions

diff --git a/compiler/optimizing/superblock_cloner.cc b/compiler/optimizing/superblock_cloner.cc
index 1e74dcba78..06459c0fb4 100644
--- a/compiler/optimizing/superblock_cloner.cc
+++ b/compiler/optimizing/superblock_cloner.cc
@@ -882,6 +882,15 @@ bool SuperblockCloner::IsSubgraphClonable() const {
     return false;
   }
 
+  // The values in live_outs should be defined in a block that dominates exit_block.
+  for (const auto& live_out : live_outs) {
+    DCHECK_EQ(exits.size(), 1u);
+    HInstruction* value = live_out.first;
+    if (!value->GetBlock()->Dominates(exits[0])) {
+      return false;
+    }
+  }
+
   return true;
 }
 
@@ -959,7 +968,8 @@ void SuperblockCloner::Run() {
     DumpInputSets();
   }
 
-  CollectLiveOutsAndCheckClonable(&live_outs_);
+  bool result = CollectLiveOutsAndCheckClonable(&live_outs_);
+  DCHECK(result);
   // Find an area in the graph for which control flow information should be adjusted.
   FindAndSetLocalAreaForAdjustments();
   ConstructSubgraphClosedSSA();
@@ -997,21 +1007,13 @@ void SuperblockCloner::CleanUp() {
   // transformation it could happen that there is such block with a phi with a single input.
   // As this is needed to be processed we also simplify phis with multiple same inputs here.
   for (auto entry : *bb_map_) {
-    HBasicBlock* orig_block = entry.first;
-    for (HInstructionIterator inst_it(orig_block->GetPhis()); !inst_it.Done(); inst_it.Advance()) {
-      HPhi* phi = inst_it.Current()->AsPhi();
-      if (ArePhiInputsTheSame(phi)) {
-        phi->ReplaceWith(phi->InputAt(0));
-        orig_block->RemovePhi(phi);
-      }
-    }
-
-    HBasicBlock* copy_block = GetBlockCopy(orig_block);
-    for (HInstructionIterator inst_it(copy_block->GetPhis()); !inst_it.Done(); inst_it.Advance()) {
-      HPhi* phi = inst_it.Current()->AsPhi();
-      if (ArePhiInputsTheSame(phi)) {
-        phi->ReplaceWith(phi->InputAt(0));
-        copy_block->RemovePhi(phi);
+    for (HBasicBlock* block : {entry.first, entry.second}) {
+      for (HInstructionIterator inst_it(block->GetPhis()); !inst_it.Done(); inst_it.Advance()) {
+        HPhi* phi = inst_it.Current()->AsPhi();
+        if (ArePhiInputsTheSame(phi)) {
+          phi->ReplaceWith(phi->InputAt(0));
+          block->RemovePhi(phi);
+        }
       }
     }
   }