From 8a43b3fad98540d6a25e85eccee11ae2d82733f3 Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Sun, 27 Sep 2015 01:47:46 +0000 Subject: [PATCH] [EH] Create removeUnwindEdge utility Summary: Factor the code that rewrites invokes to calls and rewrites WinEH terminators to their "unwind to caller" equivalents into a helper in Utils/Local, and use it in the three places I'm aware of that need to do this. Reviewers: andrew.w.kaylor, majnemer, rnk Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D13152 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@248677 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Transforms/Utils/Local.h | 8 ++ lib/CodeGen/WinEHPrepare.cpp | 14 +++ lib/Transforms/Utils/Local.cpp | 37 ++++++ lib/Transforms/Utils/SimplifyCFG.cpp | 117 +++--------------- test/CodeGen/WinEH/wineh-cloning.ll | 31 +++++ .../SimplifyCFG/wineh-unreachable.ll | 88 +++++++++++++ 6 files changed, 196 insertions(+), 99 deletions(-) create mode 100644 test/Transforms/SimplifyCFG/wineh-unreachable.ll diff --git a/include/llvm/Transforms/Utils/Local.h b/include/llvm/Transforms/Utils/Local.h index 11c181cbbe0..2354e0ae160 100644 --- a/include/llvm/Transforms/Utils/Local.h +++ b/include/llvm/Transforms/Utils/Local.h @@ -277,6 +277,14 @@ DbgDeclareInst *FindAllocaDbgDeclare(Value *V); bool replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress, DIBuilder &Builder, bool Deref); +/// Replace 'BB's terminator with one that does not have an unwind successor +/// block. Rewrites `invoke` to `call`, `catchendpad unwind label %foo` to +/// `catchendpad unwind to caller`, etc. Updates any PHIs in unwind successor. +/// +/// \param BB Block whose terminator will be replaced. Its terminator must +/// have an unwind successor. +void removeUnwindEdge(BasicBlock *BB); + /// \brief Remove all blocks that can not be reached from the function's entry. /// /// Returns true if any basic block was removed. diff --git a/lib/CodeGen/WinEHPrepare.cpp b/lib/CodeGen/WinEHPrepare.cpp index d5af24ed71a..b062b1a29d3 100644 --- a/lib/CodeGen/WinEHPrepare.cpp +++ b/lib/CodeGen/WinEHPrepare.cpp @@ -3184,9 +3184,23 @@ void WinEHPrepare::removeImplausibleTerminators(Function &F) { for (BasicBlock *SuccBB : TI->successors()) SuccBB->removePredecessor(BB); + if (IsUnreachableCleanupendpad) { + // We can't simply replace a cleanupendpad with unreachable, because + // its predecessor edges are EH edges and unreachable is not an EH + // pad. Change all predecessors to the "unwind to caller" form. + for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); + PI != PE;) { + BasicBlock *Pred = *PI++; + removeUnwindEdge(Pred); + } + } + new UnreachableInst(BB->getContext(), TI); TI->eraseFromParent(); } + // FIXME: Check for invokes/cleanuprets/cleanupendpads which unwind to + // implausible catchendpads (i.e. catchendpad not in immediate parent + // funclet). } } } diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index 235973c4fbc..fec7176e9a2 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -1258,6 +1258,43 @@ static bool markAliveBlocks(Function &F, return Changed; } +void llvm::removeUnwindEdge(BasicBlock *BB) { + TerminatorInst *TI = BB->getTerminator(); + + if (auto *II = dyn_cast(TI)) { + changeToCall(II); + return; + } + + TerminatorInst *NewTI; + BasicBlock *UnwindDest; + + if (auto *CRI = dyn_cast(TI)) { + NewTI = CleanupReturnInst::Create(CRI->getCleanupPad(), nullptr, CRI); + UnwindDest = CRI->getUnwindDest(); + } else if (auto *CEP = dyn_cast(TI)) { + NewTI = CleanupEndPadInst::Create(CEP->getCleanupPad(), nullptr, CEP); + UnwindDest = CEP->getUnwindDest(); + } else if (auto *CEP = dyn_cast(TI)) { + NewTI = CatchEndPadInst::Create(CEP->getContext(), nullptr, CEP); + UnwindDest = CEP->getUnwindDest(); + } else if (auto *TPI = dyn_cast(TI)) { + SmallVector TerminatePadArgs; + for (Value *Operand : TPI->arg_operands()) + TerminatePadArgs.push_back(Operand); + NewTI = TerminatePadInst::Create(TPI->getContext(), nullptr, + TerminatePadArgs, TPI); + UnwindDest = TPI->getUnwindDest(); + } else { + llvm_unreachable("Could not find unwind successor"); + } + + NewTI->takeName(TI); + NewTI->setDebugLoc(TI->getDebugLoc()); + UnwindDest->removePredecessor(BB); + TI->eraseFromParent(); +} + /// removeUnreachableBlocksFromFn - Remove blocks that are not reachable, even /// if they are in a dead cycle. Return true if a change was made, false /// otherwise. diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index 2c737599bce..403356931ca 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -2901,31 +2901,6 @@ static bool SimplifyBranchOnICmpChain(BranchInst *BI, IRBuilder<> &Builder, return true; } -// FIXME: This seems like a pretty common thing to want to do. Consider -// whether there is a more accessible place to put this. -static void convertInvokeToCall(InvokeInst *II) { - SmallVector Args(II->op_begin(), II->op_end() - 3); - // Insert a call instruction before the invoke. - CallInst *Call = CallInst::Create(II->getCalledValue(), Args, "", II); - Call->takeName(II); - Call->setCallingConv(II->getCallingConv()); - Call->setAttributes(II->getAttributes()); - Call->setDebugLoc(II->getDebugLoc()); - - // Anything that used the value produced by the invoke instruction now uses - // the value produced by the call instruction. Note that we do this even - // for void functions and calls with no uses so that the callgraph edge is - // updated. - II->replaceAllUsesWith(Call); - II->getUnwindDest()->removePredecessor(II->getParent()); - - // Insert a branch to the normal destination right before the invoke. - BranchInst::Create(II->getNormalDest(), II); - - // Finally, delete the invoke instruction! - II->eraseFromParent(); -} - bool SimplifyCFGOpt::SimplifyResume(ResumeInst *RI, IRBuilder<> &Builder) { // If this is a trivial landing pad that just continues unwinding the caught // exception then zap the landing pad, turning its invokes into calls. @@ -2944,8 +2919,8 @@ bool SimplifyCFGOpt::SimplifyResume(ResumeInst *RI, IRBuilder<> &Builder) { // Turn all invokes that unwind here into calls and delete the basic block. for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE;) { - InvokeInst *II = cast((*PI++)->getTerminator()); - convertInvokeToCall(II); + BasicBlock *Pred = *PI++; + removeUnwindEdge(Pred); } // The landingpad is now unreachable. Zap it. @@ -3056,62 +3031,11 @@ bool SimplifyCFGOpt::SimplifyCleanupReturn(CleanupReturnInst *RI) { for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE;) { // The iterator must be updated here because we are removing this pred. BasicBlock *PredBB = *PI++; - TerminatorInst *TI = PredBB->getTerminator(); if (UnwindDest == nullptr) { - if (auto *II = dyn_cast(TI)) { - // The cleanup return being simplified continues to the caller and this - // predecessor terminated with an invoke instruction. Convert the - // invoke to a call. - // This call updates the predecessor/successor chain. - convertInvokeToCall(II); - } else { - // In the remaining cases the predecessor's terminator unwinds to the - // block we are removing. We need to create a new instruction that - // unwinds to the caller. Simply setting the unwind destination to - // nullptr would leave the objects internal data in an inconsistent - // state. - // FIXME: Consider whether it is better to update setUnwindDest to - // keep things consistent. - if (auto *CRI = dyn_cast(TI)) { - auto *NewCRI = CleanupReturnInst::Create(CRI->getCleanupPad(), - nullptr, CRI); - NewCRI->takeName(CRI); - NewCRI->setDebugLoc(CRI->getDebugLoc()); - CRI->eraseFromParent(); - } else if (auto *CEP = dyn_cast(TI)) { - auto *NewCEP = CatchEndPadInst::Create(CEP->getContext(), nullptr, - CEP); - NewCEP->takeName(CEP); - NewCEP->setDebugLoc(CEP->getDebugLoc()); - CEP->eraseFromParent(); - } else if (auto *TPI = dyn_cast(TI)) { - SmallVector TerminatePadArgs; - for (Value *Operand : TPI->arg_operands()) - TerminatePadArgs.push_back(Operand); - auto *NewTPI = TerminatePadInst::Create(TPI->getContext(), nullptr, - TerminatePadArgs, TPI); - NewTPI->takeName(TPI); - NewTPI->setDebugLoc(TPI->getDebugLoc()); - TPI->eraseFromParent(); - } else { - llvm_unreachable("Unexpected predecessor to cleanup pad."); - } - } + removeUnwindEdge(PredBB); } else { - // If the predecessor did not terminate with an invoke instruction, it - // must be some variety of EH pad. TerminatorInst *TI = PredBB->getTerminator(); - // FIXME: Introducing an EH terminator base class would simplify this. - if (auto *II = dyn_cast(TI)) - II->setUnwindDest(UnwindDest); - else if (auto *CRI = dyn_cast(TI)) - CRI->setUnwindDest(UnwindDest); - else if (auto *CEP = dyn_cast(TI)) - CEP->setUnwindDest(UnwindDest); - else if (auto *TPI = dyn_cast(TI)) - TPI->setUnwindDest(UnwindDest); - else - llvm_unreachable("Unexpected predecessor to cleanup pad."); + TI->replaceUsesOfWith(BB, UnwindDest); } } @@ -3249,26 +3173,21 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) { --i; --e; Changed = true; } - } else if (InvokeInst *II = dyn_cast(TI)) { - if (II->getUnwindDest() == BB) { - // Convert the invoke to a call instruction. This would be a good - // place to note that the call does not throw though. - BranchInst *BI = Builder.CreateBr(II->getNormalDest()); - II->removeFromParent(); // Take out of symbol table - - // Insert the call now... - SmallVector Args(II->op_begin(), II->op_end()-3); - Builder.SetInsertPoint(BI); - CallInst *CI = Builder.CreateCall(II->getCalledValue(), - Args, II->getName()); - CI->setCallingConv(II->getCallingConv()); - CI->setAttributes(II->getAttributes()); - // If the invoke produced a value, the call does now instead. - II->replaceAllUsesWith(CI); - delete II; - Changed = true; - } + } else if ((isa(TI) && + cast(TI)->getUnwindDest() == BB) || + isa(TI) || isa(TI)) { + removeUnwindEdge(TI->getParent()); + Changed = true; + } else if (isa(TI) || isa(TI) || + isa(TI)) { + new UnreachableInst(TI->getContext(), TI); + TI->eraseFromParent(); + Changed = true; } + // TODO: If TI is a CatchPadInst, then (BB must be its normal dest and) + // we can eliminate it, redirecting its preds to its unwind successor, + // or to the next outer handler if the removed catch is the last for its + // catchendpad. } // If this block is now dead, remove it. diff --git a/test/CodeGen/WinEH/wineh-cloning.ll b/test/CodeGen/WinEH/wineh-cloning.ll index 4500f96e6f6..1ed71ef8375 100644 --- a/test/CodeGen/WinEH/wineh-cloning.ll +++ b/test/CodeGen/WinEH/wineh-cloning.ll @@ -421,3 +421,34 @@ unreachable: ; CHECK-NEXT: catchendpad unwind to caller ; CHECK: exit: ; CHECK-NEXT: ret void + +define void @test11() personality i32 (...)* @__CxxFrameHandler3 { +entry: + invoke void @f() + to label %exit unwind label %cleanup.outer +cleanup.outer: + %outer = cleanuppad [] + invoke void @f() + to label %outer.cont unwind label %cleanup.inner +outer.cont: + br label %merge +cleanup.inner: + %inner = cleanuppad [] + br label %merge +merge: + invoke void @f() + to label %unreachable unwind label %merge.end +unreachable: + unreachable +merge.end: + cleanupendpad %outer unwind to caller +exit: + ret void +} +; merge.end will get cloned for outer and inner, but is implausible +; from inner, so the invoke @f() in inner's copy of merge should be +; rewritten to call @f() +; CHECK-LABEL: define void @test11() +; CHECK: %inner = cleanuppad [] +; CHECK-NEXT: call void @f() +; CHECK-NEXT: unreachable diff --git a/test/Transforms/SimplifyCFG/wineh-unreachable.ll b/test/Transforms/SimplifyCFG/wineh-unreachable.ll new file mode 100644 index 00000000000..7db883b2412 --- /dev/null +++ b/test/Transforms/SimplifyCFG/wineh-unreachable.ll @@ -0,0 +1,88 @@ +; RUN: opt -S -simplifycfg < %s | FileCheck %s + +declare void @Personality() +declare void @f() + +; CHECK-LABEL: define void @test1() +define void @test1() personality i8* bitcast (void ()* @Personality to i8*) { +entry: + ; CHECK: call void @f() + invoke void @f() + to label %exit unwind label %unreachable.unwind +exit: + ret void +unreachable.unwind: + cleanuppad [] + unreachable +} + +; CHECK-LABEL: define void @test2() +define void @test2() personality i8* bitcast (void ()* @Personality to i8*) { +entry: + invoke void @f() + to label %exit unwind label %catch.pad +catch.pad: + ; CHECK: catchpad [] + ; CHECK-NEXT: to label %catch.body unwind label %catch.end + %catch = catchpad [] + to label %catch.body unwind label %catch.end +catch.body: + ; CHECK: catch.body: + ; CHECK-NEXT: call void @f() + ; CHECK-NEXT: unreachable + call void @f() + catchret %catch to label %unreachable +catch.end: + ; CHECK: catch.end: + ; CHECK-NEXT: catchendpad unwind to caller + catchendpad unwind label %unreachable.unwind +exit: + ret void +unreachable.unwind: + cleanuppad [] + unreachable +unreachable: + unreachable +} + +; CHECK-LABEL: define void @test3() +define void @test3() personality i8* bitcast (void ()* @Personality to i8*) { +entry: + invoke void @f() + to label %exit unwind label %cleanup.pad +cleanup.pad: + ; CHECK: %cleanup = cleanuppad [] + ; CHECK-NEXT: call void @f() + ; CHECK-NEXT: unreachable + %cleanup = cleanuppad [] + invoke void @f() + to label %cleanup.ret unwind label %cleanup.end +cleanup.ret: + ; This cleanupret should be rewritten to unreachable, + ; and merged into the pred block. + cleanupret %cleanup unwind label %unreachable.unwind +cleanup.end: + ; This cleanupendpad should be rewritten to unreachable, + ; causing the invoke to be rewritten to a call. + cleanupendpad %cleanup unwind label %unreachable.unwind +exit: + ret void +unreachable.unwind: + cleanuppad [] + unreachable +} + +; CHECK-LABEL: define void @test4() +define void @test4() personality i8* bitcast (void ()* @Personality to i8*) { +entry: + invoke void @f() + to label %exit unwind label %terminate.pad +terminate.pad: + ; CHECK: terminatepad [] unwind to caller + terminatepad [] unwind label %unreachable.unwind +exit: + ret void +unreachable.unwind: + cleanuppad [] + unreachable +} -- 2.34.1