Index: include/llvm/Support/GenericDomTreeConstruction.h =================================================================== --- include/llvm/Support/GenericDomTreeConstruction.h +++ include/llvm/Support/GenericDomTreeConstruction.h @@ -914,7 +914,12 @@ if (!FromTN) return; const TreeNodePtr ToTN = DT.getNode(To); - assert(ToTN && "To already unreachable -- there is no edge to delete"); + if (!ToTN) { + DEBUG(dbgs() << "\tTo (" << BlockNamePrinter(To) + << ") already unreachable -- there is no edge to delete\n"); + return; + } + const NodePtr NCDBlock = DT.findNearestCommonDominator(From, To); const TreeNodePtr NCD = DT.getNode(NCDBlock); Index: lib/Transforms/Scalar/ADCE.cpp =================================================================== --- lib/Transforms/Scalar/ADCE.cpp +++ lib/Transforms/Scalar/ADCE.cpp @@ -27,6 +27,7 @@ #include "llvm/IR/BasicBlock.h" #include "llvm/IR/CFG.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/Dominators.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/InstIterator.h" #include "llvm/IR/Instructions.h" @@ -89,6 +90,10 @@ class AggressiveDeadCodeElimination { Function &F; + + // ADCE does not use DominatorTree per se, but it updates it to preserve the + // analysis. + DominatorTree &DT; PostDominatorTree &PDT; /// Mapping of blocks to associated information, an element in BlockInfoVec. @@ -157,9 +162,10 @@ void makeUnconditional(BasicBlock *BB, BasicBlock *Target); public: - AggressiveDeadCodeElimination(Function &F, PostDominatorTree &PDT) - : F(F), PDT(PDT) {} - bool performDeadCodeElimination(); + AggressiveDeadCodeElimination(Function &F, DominatorTree &DT, + PostDominatorTree &PDT) + : F(F), DT(DT), PDT(PDT) {} + bool performDeadCodeElimination(); }; } @@ -557,14 +563,31 @@ } assert((PreferredSucc && PreferredSucc->PostOrder > 0) && "Failed to find safe successor for dead branch"); + + // Collect removed successors to update the (Post)DominatorTrees. + SmallPtrSet RemovedSuccessors; bool First = true; for (auto *Succ : successors(BB)) { - if (!First || Succ != PreferredSucc->BB) + if (!First || Succ != PreferredSucc->BB) { Succ->removePredecessor(BB); - else + RemovedSuccessors.insert(Succ); + } else First = false; } makeUnconditional(BB, PreferredSucc->BB); + + // Inform the dominators about the deleted CFG edges. + for (auto *Succ : RemovedSuccessors) { + // It might have happened that the same successor appeared multiple times + // and the CFG edge wasn't really removed. + if (Succ != PreferredSucc->BB) { + DEBUG(dbgs() << "ADCE: Removing (Post)DomTree edge " << BB->getName() + << " -> " << Succ->getName() << "\n"); + DT.deleteEdge(BB, Succ); + PDT.deleteEdge(BB, Succ); + } + } + NumBranchesRemoved += 1; } } @@ -609,6 +632,9 @@ InstInfo[NewTerm].Live = true; if (const DILocation *DL = PredTerm->getDebugLoc()) NewTerm->setDebugLoc(DL); + + InstInfo.erase(PredTerm); + PredTerm->eraseFromParent(); } //===----------------------------------------------------------------------===// @@ -617,13 +643,16 @@ // //===----------------------------------------------------------------------===// PreservedAnalyses ADCEPass::run(Function &F, FunctionAnalysisManager &FAM) { + auto &DT = FAM.getResult(F); auto &PDT = FAM.getResult(F); - if (!AggressiveDeadCodeElimination(F, PDT).performDeadCodeElimination()) + if (!AggressiveDeadCodeElimination(F, DT, PDT).performDeadCodeElimination()) return PreservedAnalyses::all(); PreservedAnalyses PA; PA.preserveSet(); PA.preserve(); + PA.preserve(); + PA.preserve(); return PA; } @@ -637,15 +666,22 @@ bool runOnFunction(Function &F) override { if (skipFunction(F)) return false; + + auto &DT = getAnalysis().getDomTree(); auto &PDT = getAnalysis().getPostDomTree(); - return AggressiveDeadCodeElimination(F, PDT).performDeadCodeElimination(); + return AggressiveDeadCodeElimination(F, DT, PDT) + .performDeadCodeElimination(); } void getAnalysisUsage(AnalysisUsage &AU) const override { + // We require DominatorTree here only to update and thus preserve it. + AU.addRequired(); AU.addRequired(); if (!RemoveControlFlowFlag) AU.setPreservesCFG(); AU.addPreserved(); + AU.addPreserved(); + AU.addPreserved(); } }; } Index: test/Transforms/ADCE/domtree-DoubleDeletion.ll =================================================================== --- /dev/null +++ test/Transforms/ADCE/domtree-DoubleDeletion.ll @@ -0,0 +1,39 @@ +; RUN: opt < %s -gvn -simplifycfg -adce | llvm-dis +; RUN: opt < %s -gvn -simplifycfg -adce -verify-dom-info | llvm-dis + +; This test makes sure that the DominatorTree properly handles +; deletion of edges that go to forward-unreachable regions. +; In this case, %land.end is already forward unreachable when +; the DT gets informed about the deletion of %entry -> %land.end. + +@a = common global i32 0, align 4 + +define i32 @main() { +entry: + %retval = alloca i32, align 4 + store i32 0, i32* %retval, align 4 + %0 = load i32, i32* @a, align 4 + %cmp = icmp ne i32 %0, 1 + br i1 %cmp, label %land.rhs, label %land.end4 + +land.rhs: ; preds = %entry + %1 = load i32, i32* @a, align 4 + %tobool = icmp ne i32 %1, 0 + br i1 %tobool, label %land.rhs1, label %land.end + +land.rhs1: ; preds = %land.rhs + br label %land.end + +land.end: ; preds = %land.rhs1, %land.rhs + %2 = phi i1 [ false, %land.rhs ], [ true, %land.rhs1 ] + %land.ext = zext i1 %2 to i32 + %conv = trunc i32 %land.ext to i16 + %conv2 = sext i16 %conv to i32 + %tobool3 = icmp ne i32 %conv2, 0 + br label %land.end4 + +land.end4: ; preds = %land.end, %entry + %3 = phi i1 [ false, %entry ], [ %tobool3, %land.end ] + %land.ext5 = zext i1 %3 to i32 + ret i32 0 +} Index: test/Transforms/ADCE/unreachable.ll =================================================================== --- /dev/null +++ test/Transforms/ADCE/unreachable.ll @@ -0,0 +1,18 @@ +; RUN: opt < %s -adce -simplifycfg | llvm-dis +; RUN: opt < %s -passes=adce | llvm-dis + +define i32 @Test(i32 %A, i32 %B) { +BB1: + br label %BB4 + +BB2: ; No predecessors! + br label %BB3 + +BB3: ; preds = %BB4, %BB2 + %ret = phi i32 [ %X, %BB4 ], [ %B, %BB2 ] ; [#uses=1] + ret i32 %ret + +BB4: ; preds = %BB1 + %X = phi i32 [ %A, %BB1 ] ; [#uses=1] + br label %BB3 +}