[LoopSimplifyCFG] Use DTU.applyUpdates instead of insert/deleteEdge

`insert/deleteEdge` methods in DTU can make updates incorrectly in some cases
(see https://bugs.llvm.org/show_bug.cgi?id=40528), and it is recommended to
use `applyUpdates` methods instead when it is needed to make a mass update in CFG.

Differential Revision: https://reviews.llvm.org/D57316
Reviewed By: kuhar

llvm-svn: 353502

lib/Transforms/Scalar/LoopSimplifyCFG.cpp

@@ -89,6 +89,8 @@ private:
   DominatorTree &DT;
   ScalarEvolution &SE;
   MemorySSAUpdater *MSSAU;
+  DomTreeUpdater DTU;
+  SmallVector<DominatorTree::UpdateType, 16> DTUpdates;
 
   // Whether or not the current loop has irreducible CFG.
   bool HasIrreducibleCFG = false;
@@ -319,14 +321,13 @@ private:
 
     // Construct split preheader and the dummy switch to thread edges from it to
     // dead exits.
-    DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Eager);
     BasicBlock *Preheader = L.getLoopPreheader();
     BasicBlock *NewPreheader = Preheader->splitBasicBlock(
         Preheader->getTerminator(),
         Twine(Preheader->getName()).concat("-split"));
-    DTU.deleteEdge(Preheader, L.getHeader());
+    DTUpdates.push_back({DominatorTree::Delete, Preheader, L.getHeader()});
-    DTU.insertEdge(NewPreheader, L.getHeader());
+    DTUpdates.push_back({DominatorTree::Insert, NewPreheader, L.getHeader()});
-    DTU.insertEdge(Preheader, NewPreheader);
+    DTUpdates.push_back({DominatorTree::Insert, Preheader, NewPreheader});
     IRBuilder<> Builder(Preheader->getTerminator());
     SwitchInst *DummySwitch =
         Builder.CreateSwitch(Builder.getInt32(0), NewPreheader);
@@ -345,7 +346,7 @@ private:
       }
       assert(DummyIdx != 0 && "Too many dead exits!");
       DummySwitch->addCase(Builder.getInt32(DummyIdx++), BB);
-      DTU.insertEdge(Preheader, BB);
+      DTUpdates.push_back({DominatorTree::Insert, Preheader, BB});
       ++NumLoopExitsDeleted;
     }
 
@@ -389,6 +390,9 @@ private:
         while (FixLCSSALoop->getParentLoop() != StillReachable)
           FixLCSSALoop = FixLCSSALoop->getParentLoop();
         assert(FixLCSSALoop && "Should be a loop!");
+        // We need all DT updates to be done before forming LCSSA.
+        DTU.applyUpdates(DTUpdates);
+        DTUpdates.clear();
         formLCSSARecursively(*FixLCSSALoop, DT, &LI, &SE);
       }
     }
@@ -397,7 +401,6 @@ private:
   /// Delete loop blocks that have become unreachable after folding. Make all
   /// relevant updates to DT and LI.
   void deleteDeadLoopBlocks() {
-    DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Eager);
     if (MSSAU) {
       SmallPtrSet<BasicBlock *, 8> DeadLoopBlocksSet(DeadLoopBlocks.begin(),
                                                      DeadLoopBlocks.end());
@@ -415,15 +418,18 @@ private:
       LI.removeBlock(BB);
     }
 
-    DeleteDeadBlocks(DeadLoopBlocks, &DTU);
+    DetatchDeadBlocks(DeadLoopBlocks, &DTUpdates);
+    DTU.applyUpdates(DTUpdates);
+    DTUpdates.clear();
+    for (auto *BB : DeadLoopBlocks)
+      BB->eraseFromParent();
+
     NumLoopBlocksDeleted += DeadLoopBlocks.size();
   }
 
   /// Constant-fold terminators of blocks acculumated in FoldCandidates into the
   /// unconditional branches.
   void foldTerminators() {
-    DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Eager);
-
     for (BasicBlock *BB : FoldCandidates) {
       assert(LI.getLoopFor(BB) == &L && "Should be a loop block!");
       BasicBlock *TheOnlySucc = getOnlyLiveSuccessor(BB);
@@ -465,7 +471,7 @@ private:
       Term->eraseFromParent();
 
       for (auto *DeadSucc : DeadSuccessors)
-        DTU.deleteEdge(BB, DeadSucc);
+        DTUpdates.push_back({DominatorTree::Delete, BB, DeadSucc});
 
       ++NumTerminatorsFolded;
     }
@@ -475,7 +481,8 @@ public:
   ConstantTerminatorFoldingImpl(Loop &L, LoopInfo &LI, DominatorTree &DT,
                                 ScalarEvolution &SE,
                                 MemorySSAUpdater *MSSAU)
-      : L(L), LI(LI), DT(DT), SE(SE), MSSAU(MSSAU) {}
+      : L(L), LI(LI), DT(DT), SE(SE), MSSAU(MSSAU),
+        DTU(DT, DomTreeUpdater::UpdateStrategy::Eager) {}
   bool run() {
     assert(L.getLoopLatch() && "Should be single latch!");
 
@@ -539,6 +546,10 @@ public:
                     << " dead blocks in loop " << L.getHeader()->getName()
                     << "\n");
       deleteDeadLoopBlocks();
+    } else {
+      // If we didn't do updates inside deleteDeadLoopBlocks, do them here.
+      DTU.applyUpdates(DTUpdates);
+      DTUpdates.clear();
     }
 
 #ifndef NDEBUG

test/Transforms/LoopSimplifyCFG/constant-fold-branch.ll

@@ -1,5 +1,4 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; XFAIL: *
 ; REQUIRES: asserts
 ; RUN: opt -S -enable-loop-simplifycfg-term-folding=true -loop-simplifycfg -debug-only=loop-simplifycfg -verify-loop-info -verify-dom-info -verify-loop-lcssa 2>&1 < %s | FileCheck %s
 ; RUN: opt -S -enable-loop-simplifycfg-term-folding=true -passes='require<domtree>,loop(simplify-cfg)' -debug-only=loop-simplifycfg -verify-loop-info -verify-dom-info -verify-loop-lcssa 2>&1 < %s | FileCheck %s
@@ -2582,6 +2581,84 @@ exit:
 }
 
 define void @test_crash_01() {
+; CHECK-LABEL: @test_crash_01(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br i1 undef, label [[BB17:%.*]], label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    switch i32 0, label [[BB2_SPLIT:%.*]] [
+; CHECK-NEXT:    i32 1, label [[BB19:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       bb2-split:
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb3:
+; CHECK-NEXT:    switch i32 undef, label [[BB16:%.*]] [
+; CHECK-NEXT:    i32 0, label [[BB15:%.*]]
+; CHECK-NEXT:    i32 1, label [[BB14:%.*]]
+; CHECK-NEXT:    i32 2, label [[BB13:%.*]]
+; CHECK-NEXT:    i32 3, label [[BB12:%.*]]
+; CHECK-NEXT:    i32 4, label [[BB11:%.*]]
+; CHECK-NEXT:    i32 5, label [[BB8:%.*]]
+; CHECK-NEXT:    i32 6, label [[BB10:%.*]]
+; CHECK-NEXT:    i32 7, label [[BB9:%.*]]
+; CHECK-NEXT:    i32 8, label [[BB7:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       bb7:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb8:
+; CHECK-NEXT:    switch i32 undef, label [[BB28:%.*]] [
+; CHECK-NEXT:    i32 0, label [[BB27:%.*]]
+; CHECK-NEXT:    i32 1, label [[BB26:%.*]]
+; CHECK-NEXT:    i32 2, label [[BB23:%.*]]
+; CHECK-NEXT:    i32 3, label [[BB24:%.*]]
+; CHECK-NEXT:    i32 4, label [[BB25:%.*]]
+; CHECK-NEXT:    i32 5, label [[BB29:%.*]]
+; CHECK-NEXT:    i32 6, label [[BB22:%.*]]
+; CHECK-NEXT:    i32 7, label [[BB20:%.*]]
+; CHECK-NEXT:    i32 8, label [[BB21:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       bb9:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb10:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb11:
+; CHECK-NEXT:    br label [[BB8]]
+; CHECK:       bb12:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb13:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb14:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb15:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb16:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb17:
+; CHECK-NEXT:    ret void
+; CHECK:       bb19:
+; CHECK-NEXT:    ret void
+; CHECK:       bb20:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb21:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb22:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb23:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb24:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb25:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb26:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb27:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb28:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb29:
+; CHECK-NEXT:    br label [[BB3]]
+;
 bb:
   br label %bb1