Skip to content
This repository has been archived by the owner on Feb 5, 2019. It is now read-only.

Commit

Permalink
[SimplifyCFG] Don't kill empty cleanuppads with multiple uses
Browse files Browse the repository at this point in the history
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
  • Loading branch information
majnemer authored and alexcrichton committed Jun 5, 2016
1 parent a73c41e commit 80ad955
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 11 deletions.
27 changes: 16 additions & 11 deletions lib/Transforms/Utils/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<PHINode>(I); ++I)
if (cast<PHINode>(I)->getIncomingValueForBlock(BB) == V) {
PHI = cast<PHINode>(I);
Expand Down Expand Up @@ -2561,7 +2561,7 @@ static bool mergeConditionalStoreToAddress(BasicBlock *PTB, BasicBlock *PFB,

QStore->eraseFromParent();
PStore->eraseFromParent();

return true;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2652,7 +2652,7 @@ static bool mergeConditionalStores(BranchInst *PBI, BranchInst *QBI) {
if (StoreInst *SI = dyn_cast<StoreInst>(&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.
Expand Down Expand Up @@ -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<DbgInfoIntrinsic>(I))
Expand Down Expand Up @@ -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<UnreachableInst>(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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down
24 changes: 24 additions & 0 deletions test/Transforms/SimplifyCFG/empty-cleanuppad.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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*)
Expand Down

0 comments on commit 80ad955

Please sign in to comment.