From d28c1bc303cdffe04b9433bb3757e331f6e250af Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Tue, 5 Jan 2016 02:37:41 +0000 Subject: [PATCH] [WinEH] Simplify unreachable catchpads Summary: At least for CoreCLR, a catchpad which immediately executes an `unreachable` instruction indicates that the exception can never have a matching type, and so such catchpads can be removed, and so can their catchswitches if the catchswitch becomes empty. Reviewers: rnk, andrew.w.kaylor, majnemer Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D15846 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256809 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/Instructions.h | 2 + lib/IR/Instructions.cpp | 11 ++ lib/Transforms/Utils/SimplifyCFG.cpp | 66 ++++++++-- test/Transforms/SimplifyCFG/empty-catchpad.ll | 115 ++++++++++++++++++ 4 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 test/Transforms/SimplifyCFG/empty-catchpad.ll diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h index d781c7af36d..1659ebf9d7c 100644 --- a/include/llvm/IR/Instructions.h +++ b/include/llvm/IR/Instructions.h @@ -3966,6 +3966,8 @@ public: /// point to the added handler. void addHandler(BasicBlock *Dest); + void removeHandler(handler_iterator HI); + unsigned getNumSuccessors() const { return getNumOperands() - 1; } BasicBlock *getSuccessor(unsigned Idx) const { assert(Idx < getNumSuccessors() && diff --git a/lib/IR/Instructions.cpp b/lib/IR/Instructions.cpp index 4ae2fd522b5..0d6d23bb45d 100644 --- a/lib/IR/Instructions.cpp +++ b/lib/IR/Instructions.cpp @@ -934,6 +934,17 @@ void CatchSwitchInst::addHandler(BasicBlock *Handler) { getOperandList()[OpNo] = Handler; } +void CatchSwitchInst::removeHandler(handler_iterator HI) { + // Move all subsequent handlers up one. + Use *EndDst = op_end() - 1; + for (Use *CurDst = HI.getCurrent(); CurDst != EndDst; ++CurDst) + *CurDst = *(CurDst + 1); + // Null out the last handler use. + *EndDst = nullptr; + + setNumHungOffUseOperands(getNumOperands() - 1); +} + BasicBlock *CatchSwitchInst::getSuccessorV(unsigned idx) const { return getSuccessor(idx); } diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index d0932f834cf..3bb3fa5a301 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -20,6 +20,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" #include "llvm/Analysis/ConstantFolding.h" +#include "llvm/Analysis/EHPersonalities.h" #include "llvm/Analysis/InstructionSimplify.h" #include "llvm/Analysis/TargetTransformInfo.h" #include "llvm/Analysis/ValueTracking.h" @@ -3448,18 +3449,26 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) { if (isa(BBI) && !isa(BBI)) break; if (BBI->mayHaveSideEffects()) { - if (StoreInst *SI = dyn_cast(BBI)) { + if (auto *SI = dyn_cast(BBI)) { if (SI->isVolatile()) break; - } else if (LoadInst *LI = dyn_cast(BBI)) { + } else if (auto *LI = dyn_cast(BBI)) { if (LI->isVolatile()) break; - } else if (AtomicRMWInst *RMWI = dyn_cast(BBI)) { + } else if (auto *RMWI = dyn_cast(BBI)) { if (RMWI->isVolatile()) break; - } else if (AtomicCmpXchgInst *CXI = dyn_cast(BBI)) { + } else if (auto *CXI = dyn_cast(BBI)) { if (CXI->isVolatile()) break; + } else if (isa(BBI)) { + // A catchpad may invoke exception object constructors and such, which + // in some languages can be arbitrary code, so be conservative by + // default. + // For CoreCLR, it just involves a type test, so can be removed. + if (classifyEHPersonality(BB->getParent()->getPersonalityFn()) != + EHPersonality::CoreCLR) + break; } else if (!isa(BBI) && !isa(BBI) && !isa(BBI)) { break; @@ -3485,7 +3494,7 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) { for (unsigned i = 0, e = Preds.size(); i != e; ++i) { TerminatorInst *TI = Preds[i]->getTerminator(); IRBuilder<> Builder(TI); - if (BranchInst *BI = dyn_cast(TI)) { + if (auto *BI = dyn_cast(TI)) { if (BI->isUnconditional()) { if (BI->getSuccessor(0) == BB) { new UnreachableInst(TI->getContext(), TI); @@ -3502,7 +3511,7 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) { Changed = true; } } - } else if (SwitchInst *SI = dyn_cast(TI)) { + } else if (auto *SI = dyn_cast(TI)) { for (SwitchInst::CaseIt i = SI->case_begin(), e = SI->case_end(); i != e; ++i) if (i.getCaseSuccessor() == BB) { @@ -3511,18 +3520,49 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) { --i; --e; Changed = true; } - } else if ((isa(TI) && - cast(TI)->getUnwindDest() == BB) || - isa(TI)) { - removeUnwindEdge(TI->getParent()); - Changed = true; + } else if (auto *II = dyn_cast(TI)) { + if (II->getUnwindDest() == BB) { + removeUnwindEdge(TI->getParent()); + Changed = true; + } + } else if (auto *CSI = dyn_cast(TI)) { + if (CSI->getUnwindDest() == BB) { + removeUnwindEdge(TI->getParent()); + Changed = true; + continue; + } + + for (CatchSwitchInst::handler_iterator I = CSI->handler_begin(), + E = CSI->handler_end(); + I != E; ++I) { + if (*I == BB) { + CSI->removeHandler(I); + --I; + --E; + Changed = true; + } + } + if (CSI->getNumHandlers() == 0) { + BasicBlock *CatchSwitchBB = CSI->getParent(); + if (CSI->hasUnwindDest()) { + // Redirect preds to the unwind dest + CatchSwitchBB->replaceAllUsesWith(CSI->getUnwindDest()); + } else { + // Rewrite all preds to unwind to caller (or from invoke to call). + SmallVector EHPreds(predecessors(CatchSwitchBB)); + for (BasicBlock *EHPred : EHPreds) + removeUnwindEdge(EHPred); + } + // The catchswitch is no longer reachable. + new UnreachableInst(CSI->getContext(), CSI); + CSI->eraseFromParent(); + Changed = true; + } } else if (isa(TI)) { new UnreachableInst(TI->getContext(), TI); TI->eraseFromParent(); Changed = true; } - // TODO: We can remove a catchswitch if all it's catchpads end in - // unreachable. } // If this block is now dead, remove it. diff --git a/test/Transforms/SimplifyCFG/empty-catchpad.ll b/test/Transforms/SimplifyCFG/empty-catchpad.ll new file mode 100644 index 00000000000..2926cd3f7dc --- /dev/null +++ b/test/Transforms/SimplifyCFG/empty-catchpad.ll @@ -0,0 +1,115 @@ +; RUN: opt < %s -simplifycfg -S | FileCheck %s + +declare void @f() +declare void @llvm.foo(i32) nounwind +declare void @ProcessCLRException() + +define void @test1() personality void ()* @ProcessCLRException { +entry: + invoke void @f() + to label %exit unwind label %exn.dispatch +exn.dispatch: + %cs = catchswitch within none [label %pad1, label %pad2] unwind to caller +pad1: + %cp1 = catchpad within %cs [i32 1] + call void @llvm.foo(i32 1) + catchret from %cp1 to label %exit +pad2: + %cp2 = catchpad within %cs [i32 2] + unreachable +exit: + ret void +} +; Remove unreachble catch2, leave catch1 as-is +; CHECK-LABEL: define void @test1() +; CHECK: %cs = catchswitch within none [label %pad1] unwind to caller +; CHECK-NOT: catchpad +; CHECK: %cp1 = catchpad within %cs [i32 1] +; CHECK-NOT: catchpad + +; Remove both catchpads and the catchswitch from exn.dispatch +; CHECK-LABEL: define void @test2() +define void @test2() personality void ()* @ProcessCLRException { +entry: + invoke void @f() + to label %via.cleanup unwind label %exn.dispatch + ; CHECK-NOT: invoke + ; CHECK: call void @f() +via.cleanup: + invoke void @f() + to label %via.catchswitch unwind label %cleanup.inner +cleanup.inner: + %cp.inner = cleanuppad within none [] + call void @llvm.foo(i32 0) + cleanupret from %cp.inner unwind label %exn.dispatch + ; CHECK: cleanupret from %cp.inner unwind to caller +via.catchswitch: + invoke void @f() + to label %exit unwind label %dispatch.inner +dispatch.inner: + %cs.inner = catchswitch within none [label %pad.inner] unwind label %exn.dispatch + ; CHECK: %cs.inner = catchswitch within none [label %pad.inner] unwind to caller +pad.inner: + %catch.inner = catchpad within %cs.inner [i32 0] + ; CHECK: %catch.inner = catchpad within %cs.inner + call void @llvm.foo(i32 1) + catchret from %catch.inner to label %exit +exn.dispatch: + %cs = catchswitch within none [label %pad1, label %pad2] unwind to caller + ; CHECK-NOT: catchswitch within + ; CHECK-NOT: catchpad +pad1: + catchpad within %cs [i32 1] + unreachable +pad2: + catchpad within %cs [i32 2] + unreachable +exit: + ret void +} + +; Same as @test2, but exn.dispatch catchswitch has an unwind dest that +; preds need to be reidrected to +; CHECK-LABEL: define void @test3() +define void @test3() personality void ()* @ProcessCLRException { +entry: + invoke void @f() + to label %via.cleanup unwind label %exn.dispatch + ; CHECK: invoke void @f() + ; CHECK-NEXT: to label %via.cleanup unwind label %cleanup +via.cleanup: + invoke void @f() + to label %via.catchswitch unwind label %cleanup.inner +cleanup.inner: + %cp.inner = cleanuppad within none [] + call void @llvm.foo(i32 0) + cleanupret from %cp.inner unwind label %exn.dispatch + ; CHECK: cleanupret from %cp.inner unwind label %cleanup +via.catchswitch: + invoke void @f() + to label %exit unwind label %dispatch.inner +dispatch.inner: + %cs.inner = catchswitch within none [label %pad.inner] unwind label %exn.dispatch + ; CHECK: %cs.inner = catchswitch within none [label %pad.inner] unwind label %cleanup +pad.inner: + %catch.inner = catchpad within %cs.inner [i32 0] + ; CHECK: %catch.inner = catchpad within %cs.inner + call void @llvm.foo(i32 1) + catchret from %catch.inner to label %exit +exn.dispatch: + %cs = catchswitch within none [label %pad1, label %pad2] unwind label %cleanup + ; CHECK-NOT: catchswitch within + ; CHECK-NOT: catchpad +pad1: + catchpad within %cs [i32 1] + unreachable +pad2: + catchpad within %cs [i32 2] + unreachable +cleanup: + %cp = cleanuppad within none [] + call void @llvm.foo(i32 0) + cleanupret from %cp unwind to caller +exit: + ret void +} -- 2.34.1