From 80ad955b60b3ac02d0462a4a65fcea597d0ebfb1 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Sat, 4 Jun 2016 23:50:03 +0000 Subject: [PATCH] [SimplifyCFG] Don't kill empty cleanuppads with multiple uses A basic block could contain: %cp = cleanuppad [] cleanupret from %cp unwind to caller This basic block is empty and is thus a candidate for removal. However, there can be other uses of %cp outside of this basic block. This is only possible in unreachable blocks. Make our transform more correct by checking that the pad has a single user before removing the BB. This fixes PR28005. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@271816 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Utils/SimplifyCFG.cpp | 27 +++++++++++-------- .../SimplifyCFG/empty-cleanuppad.ll | 24 +++++++++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index e484b690597e..84f533277da5 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -2417,7 +2417,7 @@ static Value *ensureValueAvailableInSuccessor(Value *V, BasicBlock *BB, // where OtherBB is the single other predecessor of BB's only successor. PHINode *PHI = nullptr; BasicBlock *Succ = BB->getSingleSuccessor(); - + for (auto I = Succ->begin(); isa(I); ++I) if (cast(I)->getIncomingValueForBlock(BB) == V) { PHI = cast(I); @@ -2561,7 +2561,7 @@ static bool mergeConditionalStoreToAddress(BasicBlock *PTB, BasicBlock *PFB, QStore->eraseFromParent(); PStore->eraseFromParent(); - + return true; } @@ -2593,7 +2593,7 @@ static bool mergeConditionalStores(BranchInst *PBI, BranchInst *QBI) { // We model triangles as a type of diamond with a nullptr "true" block. // Triangles are canonicalized so that the fallthrough edge is represented by // a true condition, as in the diagram above. - // + // BasicBlock *PTB = PBI->getSuccessor(0); BasicBlock *PFB = PBI->getSuccessor(1); BasicBlock *QTB = QBI->getSuccessor(0); @@ -2652,7 +2652,7 @@ static bool mergeConditionalStores(BranchInst *PBI, BranchInst *QBI) { if (StoreInst *SI = dyn_cast(&I)) QStoreAddresses.insert(SI->getPointerOperand()); } - + set_intersect(PStoreAddresses, QStoreAddresses); // set_intersect mutates PStoreAddresses in place. Rename it here to make it // clear what it contains. @@ -3381,7 +3381,12 @@ bool SimplifyCFGOpt::SimplifyCleanupReturn(CleanupReturnInst *RI) { // This isn't an empty cleanup. return false; - // Check that there are no other instructions except for debug intrinsics. + // We cannot kill the pad if it has multiple uses. This typically arises + // from unreachable basic blocks. + if (!CPInst->hasOneUse()) + return false; + + // Check that there are no other instructions except for benign intrinsics. BasicBlock::iterator I = CPInst->getIterator(), E = RI->getIterator(); while (++I != E) if (!isa(I)) @@ -3818,17 +3823,17 @@ static bool EliminateDeadSwitchCases(SwitchInst *SI, AssumptionCache *AC, } } - // If we can prove that the cases must cover all possible values, the - // default destination becomes dead and we can remove it. If we know some + // If we can prove that the cases must cover all possible values, the + // default destination becomes dead and we can remove it. If we know some // of the bits in the value, we can use that to more precisely compute the // number of possible unique case values. bool HasDefault = !isa(SI->getDefaultDest()->getFirstNonPHIOrDbg()); - const unsigned NumUnknownBits = Bits - + const unsigned NumUnknownBits = Bits - (KnownZero.Or(KnownOne)).countPopulation(); assert(NumUnknownBits <= Bits); if (HasDefault && DeadCases.empty() && - NumUnknownBits < 64 /* avoid overflow */ && + NumUnknownBits < 64 /* avoid overflow */ && SI->getNumCases() == (1ULL << NumUnknownBits)) { DEBUG(dbgs() << "SimplifyCFG: switch default is dead.\n"); BasicBlock *NewDefault = SplitBlockPredecessors(SI->getDefaultDest(), @@ -4584,7 +4589,7 @@ static void reuseTableCompare(User *PhiUser, BasicBlock *PhiBlock, assert((CaseConst == TrueConst || CaseConst == FalseConst) && "Expect true or false as compare result."); } - + // Check if the branch instruction dominates the phi node. It's a simple // dominance check, but sufficient for our needs. // Although this check is invariant in the calling loops, it's better to do it @@ -5149,7 +5154,7 @@ bool SimplifyCFGOpt::SimplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) { if (PBI != BI && PBI->isConditional()) if (mergeConditionalStores(PBI, BI)) return SimplifyCFG(BB, TTI, BonusInstThreshold, AC) | true; - + return false; } diff --git a/test/Transforms/SimplifyCFG/empty-cleanuppad.ll b/test/Transforms/SimplifyCFG/empty-cleanuppad.ll index 57b362889955..e93802ee266c 100644 --- a/test/Transforms/SimplifyCFG/empty-cleanuppad.ll +++ b/test/Transforms/SimplifyCFG/empty-cleanuppad.ll @@ -405,6 +405,30 @@ return: ; preds = %invoke.cont, %catch ret void } +; CHECK-LABEL: define void @f10( +define void @f10(i32 %V) personality i32 (...)* @__CxxFrameHandler3 { +entry: + invoke void @g() + to label %unreachable unwind label %cleanup +; CHECK: call void @g() +; CHECK-NEXT: unreachable + +unreachable: + unreachable + +cleanup: + %cp = cleanuppad within none [] + switch i32 %V, label %cleanupret1 [ + i32 0, label %cleanupret2 + ] + +cleanupret1: + cleanupret from %cp unwind to caller + +cleanupret2: + cleanupret from %cp unwind to caller +} + %struct.S = type { i8 } %struct.S2 = type { i8 } declare void @"\01??1S2@@QEAA@XZ"(%struct.S2*)