From eed2a938a518054786f0b353b488fd83f854a917 Mon Sep 17 00:00:00 2001 From: Max Kazantsev Date: Fri, 8 Feb 2019 08:12:41 +0000 Subject: [PATCH] [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 | 33 +++-- .../LoopSimplifyCFG/constant-fold-branch.ll | 119 ++++++++++++++---- 2 files changed, 120 insertions(+), 32 deletions(-) diff --git a/lib/Transforms/Scalar/LoopSimplifyCFG.cpp b/lib/Transforms/Scalar/LoopSimplifyCFG.cpp index dbe68d00c50..01d770ca515 100644 --- a/lib/Transforms/Scalar/LoopSimplifyCFG.cpp +++ b/lib/Transforms/Scalar/LoopSimplifyCFG.cpp @@ -89,6 +89,8 @@ private: DominatorTree &DT; ScalarEvolution &SE; MemorySSAUpdater *MSSAU; + DomTreeUpdater DTU; + SmallVector 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()); - DTU.insertEdge(NewPreheader, L.getHeader()); - DTU.insertEdge(Preheader, NewPreheader); + DTUpdates.push_back({DominatorTree::Delete, Preheader, L.getHeader()}); + DTUpdates.push_back({DominatorTree::Insert, NewPreheader, L.getHeader()}); + 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 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 diff --git a/test/Transforms/LoopSimplifyCFG/constant-fold-branch.ll b/test/Transforms/LoopSimplifyCFG/constant-fold-branch.ll index 4e75910b433..d89193edf93 100644 --- a/test/Transforms/LoopSimplifyCFG/constant-fold-branch.ll +++ b/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,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 @@ -2596,21 +2673,21 @@ bb3: ; preds = %bb6, %bb2 bb4: ; preds = %bb3 switch i32 0, label %bb5 [ - i32 1, label %bb19 - i32 2, label %bb18 + i32 1, label %bb19 + i32 2, label %bb18 ] bb5: ; preds = %bb4 switch i32 undef, label %bb16 [ - i32 0, label %bb15 - i32 1, label %bb14 - i32 2, label %bb13 - i32 3, label %bb12 - i32 4, label %bb11 - i32 5, label %bb8 - i32 6, label %bb10 - i32 7, label %bb9 - i32 8, label %bb7 + i32 0, label %bb15 + i32 1, label %bb14 + i32 2, label %bb13 + i32 3, label %bb12 + i32 4, label %bb11 + i32 5, label %bb8 + i32 6, label %bb10 + i32 7, label %bb9 + i32 8, label %bb7 ] bb6: ; preds = %bb29, %bb18 @@ -2621,15 +2698,15 @@ bb7: ; preds = %bb5 bb8: ; preds = %bb11, %bb5 switch i32 undef, label %bb28 [ - i32 0, label %bb27 - i32 1, label %bb26 - i32 2, label %bb23 - i32 3, label %bb24 - i32 4, label %bb25 - i32 5, label %bb29 - i32 6, label %bb22 - i32 7, label %bb20 - i32 8, label %bb21 + i32 0, label %bb27 + i32 1, label %bb26 + i32 2, label %bb23 + i32 3, label %bb24 + i32 4, label %bb25 + i32 5, label %bb29 + i32 6, label %bb22 + i32 7, label %bb20 + i32 8, label %bb21 ] bb9: ; preds = %bb5