[PATCH] D56284: [UnrollRuntime] Fix domTree failure in multiexit unrolling

[Anna Thomas via Phabricator via llvm-commits]

anna created this revision.
anna added reviewers: reames, mzolotukhin, mkazantsev, hfinkel.
Herald added subscribers: dmgreen, zzheng.

This fixes the IDom for an exit block when runtime unrolling under multiexit/exiting case.
We initially had a restrictive check that the IDom is only updated when
it is the header of the loop.
However, we also need to update the IDom to the correct one when the
IDom is any block within the original loop. See added test case (which
fails dom tree verification without the patch).


Repository:
  rL LLVM

https://reviews.llvm.org/D56284

Files:
  lib/Transforms/Utils/LoopUnrollRuntime.cpp
  test/Transforms/LoopUnroll/runtime-loop-multiexit-dom-verify.ll


Index: test/Transforms/LoopUnroll/runtime-loop-multiexit-dom-verify.ll
===================================================================
--- test/Transforms/LoopUnroll/runtime-loop-multiexit-dom-verify.ll
+++ test/Transforms/LoopUnroll/runtime-loop-multiexit-dom-verify.ll
@@ -124,3 +124,50 @@
 exitsucc:                                              ; preds = %headerexit
   ret i64 96
 }
+
+; exit block (%default) has an exiting block and another exit block as predecessors.
+define void @test4(i16 %c3) {
+; CHECK-LABEL: test4
+
+; CHECK-LABEL: exiting.prol:
+; CHECK-NEXT:   switch i16 %c3, label %default.loopexit.loopexit1 [
+
+; CHECK-LABEL: exiting:
+; CHECK-NEXT:   switch i16 %c3, label %default.loopexit.loopexit [
+
+; CHECK-LABEL: default.loopexit.loopexit:
+; CHECK-NEXT:   br label %default.loopexit
+
+; CHECK-LABEL: default.loopexit.loopexit1:
+; CHECK-NEXT:   br label %default.loopexit
+
+; CHECK-LABEL: default.loopexit:
+; CHECK-NEXT:   br label %default
+preheader:
+  %c1 = zext i32 undef to i64
+  br label %header
+
+header:                                       ; preds = %latch, %preheader
+  %indvars.iv = phi i64 [ 0, %preheader ], [ %indvars.iv.next, %latch ]
+  br label %exiting
+
+exiting:                                           ; preds = %header
+  switch i16 %c3, label %default [
+    i16 45, label %otherexit
+    i16 95, label %latch
+  ]
+
+latch:                                          ; preds = %exiting
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %c2 = icmp ult i64 %indvars.iv.next, %c1
+  br i1 %c2, label %header, label %latchexit
+
+latchexit:                                          ; preds = %latch
+  ret void
+
+default:                                          ; preds = %otherexit, %exiting
+  ret void
+
+otherexit:                                           ; preds = %exiting
+  br label %default
+}
Index: lib/Transforms/Utils/LoopUnrollRuntime.cpp
===================================================================
--- lib/Transforms/Utils/LoopUnrollRuntime.cpp
+++ lib/Transforms/Utils/LoopUnrollRuntime.cpp
@@ -849,16 +849,18 @@
    // the remainder code), we set the immediate dominator as the preheader.
    if (DT) {
      DT->changeImmediateDominator(BB, PreHeader);
-     // Also update the IDom for immediate successors of BB.  If the current
-     // IDom is the header, update the IDom to be the preheader because that is
-     // the nearest common dominator of all predecessors of SuccBB.  We need to
-     // check for IDom being the header because successors of exit blocks can
-     // have edges from outside the loop, and we should not incorrectly update
-     // the IDom in that case.
+     // Also update the IDom for immediate successors of BB to the Preheader,
+     // where applicable.
      for (BasicBlock *SuccBB: successors(BB))
        if (ImmediateSuccessorsOfExitBlocks.insert(SuccBB).second) {
-         if (DT->getNode(SuccBB)->getIDom()->getBlock() == Header) {
-           assert(!SuccBB->getSinglePredecessor() &&
+         // If the current IDom of SuccBB is within the loop, update the IDom to
+         // be the preheader because that is the nearest common dominator of all
+         // predecessors of SuccBB (i.e. BB and other blocks from the loop).
+         // We need to check for IDom being in the loop because successors of
+         // exit blocks can have edges from outside the loop, and we should not
+         // incorrectly update the IDom in that case.
+         if (LI->getLoopFor(DT->getNode(SuccBB)->getIDom()->getBlock()) == L) {
+           assert(!SuccBB->getUniquePredecessor() &&
                   "BB should be the IDom then!");
            DT->changeImmediateDominator(SuccBB, PreHeader);
          }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56284.180116.patch
Type: text/x-patch
Size: 3743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190103/3cff605f/attachment.bin>