From 925ddf1a93d25fb11452d8e08042a2a9f5337e78 Mon Sep 17 00:00:00 2001 From: Balaram Makam Date: Wed, 25 Oct 2017 14:55:48 +0000 Subject: [PATCH] [Local] Fix a bug in the domtree update logic for MergeBasicBlockIntoOnlyPred. Summary: For some irreducible CFG the domtree nodes might be dead, do not update domtree for dead nodes. Reviewers: kuhar, dberlin, hfinkel Reviewed By: kuhar Subscribers: llvm-commits, mcrosier Differential Revision: https://reviews.llvm.org/D38960 llvm-svn: 316582 --- llvm/lib/Transforms/Utils/Local.cpp | 10 ++++--- llvm/unittests/Transforms/Utils/Local.cpp | 45 +++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index fd33677..8c643c9 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -649,9 +649,13 @@ void llvm::MergeBasicBlockIntoOnlyPred(BasicBlock *DestBB, DominatorTree *DT) { DestBB->moveAfter(PredBB); if (DT) { - BasicBlock *PredBBIDom = DT->getNode(PredBB)->getIDom()->getBlock(); - DT->changeImmediateDominator(DestBB, PredBBIDom); - DT->eraseNode(PredBB); + // For some irreducible CFG we end up having forward-unreachable blocks + // so check if getNode returns a valid node before updating the domtree. + if (DomTreeNode *DTN = DT->getNode(PredBB)) { + BasicBlock *PredBBIDom = DTN->getIDom()->getBlock(); + DT->changeImmediateDominator(DestBB, PredBBIDom); + DT->eraseNode(PredBB); + } } // Nuke BB. PredBB->eraseFromParent(); diff --git a/llvm/unittests/Transforms/Utils/Local.cpp b/llvm/unittests/Transforms/Utils/Local.cpp index 1f58dc8..ee864e6 100644 --- a/llvm/unittests/Transforms/Utils/Local.cpp +++ b/llvm/unittests/Transforms/Utils/Local.cpp @@ -166,3 +166,48 @@ TEST(Local, ReplaceDbgDeclare) { Declares++; EXPECT_EQ(2, Declares); } + +/// Build the dominator tree for the function and run the Test. +static void runWithDomTree( + Module &M, StringRef FuncName, + function_ref Test) { + auto *F = M.getFunction(FuncName); + ASSERT_NE(F, nullptr) << "Could not find " << FuncName; + // Compute the dominator tree for the function. + DominatorTree DT(*F); + Test(*F, &DT); +} + +TEST(Local, MergeBasicBlockIntoOnlyPred) { + LLVMContext C; + + std::unique_ptr M = parseIR( + C, + "define i32 @f(i8* %str) {\n" + "entry:\n" + " br label %bb2.i\n" + "bb2.i: ; preds = %bb4.i, %entry\n" + " br i1 false, label %bb4.i, label %base2flt.exit204\n" + "bb4.i: ; preds = %bb2.i\n" + " br i1 false, label %base2flt.exit204, label %bb2.i\n" + "bb10.i196.bb7.i197_crit_edge: ; No predecessors!\n" + " br label %bb7.i197\n" + "bb7.i197: ; preds = %bb10.i196.bb7.i197_crit_edge\n" + " %.reg2mem.0 = phi i32 [ %.reg2mem.0, %bb10.i196.bb7.i197_crit_edge ]\n" + " br i1 undef, label %base2flt.exit204, label %base2flt.exit204\n" + "base2flt.exit204: ; preds = %bb7.i197, %bb7.i197, %bb2.i, %bb4.i\n" + " ret i32 0\n" + "}\n"); + runWithDomTree( + *M, "f", [&](Function &F, DominatorTree *DT) { + for (Function::iterator I = F.begin(), E = F.end(); I != E;) { + BasicBlock *BB = &*I++; + BasicBlock *SinglePred = BB->getSinglePredecessor(); + if (!SinglePred || SinglePred == BB || BB->hasAddressTaken()) continue; + BranchInst *Term = dyn_cast(SinglePred->getTerminator()); + if (Term && !Term->isConditional()) + MergeBasicBlockIntoOnlyPred(BB, DT); + } + EXPECT_TRUE(DT->verify()); + }); +} -- 2.7.4