From de17e7736f6ca8c708acfcc297a466e786f38833 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Sat, 15 Aug 2015 02:46:08 +0000 Subject: [PATCH] [IR] Give catchret an optional 'return value' operand Some personality routines require funclet exit points to be clearly marked, this is done by producing a token at the funclet pad and consuming it at the corresponding ret instruction. CleanupReturnInst already had a spot for this operand but CatchReturnInst did not. Other personality routines don't need to use this which is why it has been made optional. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@245149 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/LangRef.rst | 3 ++- include/llvm/IR/Instructions.h | 34 +++++++++++++++++------- lib/Analysis/CaptureTracking.cpp | 9 ++++--- lib/Analysis/InstructionSimplify.cpp | 4 +-- lib/Analysis/ScalarEvolutionExpander.cpp | 2 ++ lib/AsmParser/LLParser.cpp | 17 +++++++++--- lib/Bitcode/Reader/BitcodeReader.cpp | 12 ++++++--- lib/Bitcode/Writer/BitcodeWriter.cpp | 2 ++ lib/IR/AsmWriter.cpp | 10 +++++++ lib/IR/Dominators.cpp | 32 ++++++++++++++-------- lib/IR/Instructions.cpp | 31 ++++++++++++++------- lib/Transforms/Scalar/Reassociate.cpp | 2 ++ lib/Transforms/Utils/CodeExtractor.cpp | 14 +++++++--- lib/Transforms/Utils/LCSSA.cpp | 11 ++++---- test/CodeGen/WinEH/wineh-demotion.ll | 16 +++++------ test/Feature/exception.ll | 11 +++++--- 16 files changed, 147 insertions(+), 63 deletions(-) diff --git a/docs/LangRef.rst b/docs/LangRef.rst index ef5913b78f3..43b47763303 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -5284,7 +5284,7 @@ Syntax: :: - catchret label + catchret to label Overview: """"""""" @@ -5308,6 +5308,7 @@ whose unwinding was interrupted with a The :ref:`personality function ` gets a chance to execute arbitrary code to, for example, run a C++ destructor. Control then transfers to ``normal``. +It may be passed an optional, personality specific, value. Example: """""""" diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h index 6fe83be4af0..a4d9f7ba9e2 100644 --- a/include/llvm/IR/Instructions.h +++ b/include/llvm/IR/Instructions.h @@ -4037,8 +4037,11 @@ class CatchReturnInst : public TerminatorInst { CatchReturnInst(const CatchReturnInst &RI); private: - CatchReturnInst(BasicBlock *BB, Instruction *InsertBefore = nullptr); - CatchReturnInst(BasicBlock *BB, BasicBlock *InsertAtEnd); + void init(BasicBlock *BB, Value *RetVal); + CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values, + Instruction *InsertBefore = nullptr); + CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values, + BasicBlock *InsertAtEnd); protected: // Note: Instruction needs to be a friend here to call cloneImpl. @@ -4046,22 +4049,35 @@ protected: CatchReturnInst *cloneImpl() const; public: - static CatchReturnInst *Create(BasicBlock *BB, + static CatchReturnInst *Create(BasicBlock *BB, Value *RetVal = nullptr, Instruction *InsertBefore = nullptr) { - return new (1) CatchReturnInst(BB, InsertBefore); + assert(BB); + unsigned Values = 1; + if (RetVal) + ++Values; + return new (Values) CatchReturnInst(BB, RetVal, Values, InsertBefore); } - static CatchReturnInst *Create(BasicBlock *BB, BasicBlock *InsertAtEnd) { - return new (1) CatchReturnInst(BB, InsertAtEnd); + static CatchReturnInst *Create(BasicBlock *BB, Value *RetVal, + BasicBlock *InsertAtEnd) { + assert(BB); + unsigned Values = 1; + if (RetVal) + ++Values; + return new (Values) CatchReturnInst(BB, RetVal, Values, InsertAtEnd); } /// Provide fast operand accessors DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value); /// Convenience accessors. - BasicBlock *getSuccessor() const { return cast(Op<0>()); } - void setSuccessor(BasicBlock *NewSucc) { Op<0>() = (Value *)NewSucc; } + BasicBlock *getSuccessor() const { return cast(Op<-1>()); } + void setSuccessor(BasicBlock *NewSucc) { Op<-1>() = (Value *)NewSucc; } unsigned getNumSuccessors() const { return 1; } + bool hasReturnValue() const { return getNumOperands() > 1; } + Value *getReturnValue() const { return Op<-2>(); } + void setReturnValue(Value *RetVal) { Op<-2>() = RetVal; } + // Methods for support type inquiry through isa, cast, and dyn_cast: static inline bool classof(const Instruction *I) { return (I->getOpcode() == Instruction::CatchRet); @@ -4078,7 +4094,7 @@ private: template <> struct OperandTraits - : public FixedNumOperandTraits {}; + : public VariadicOperandTraits {}; DEFINE_TRANSPARENT_OPERAND_ACCESSORS(CatchReturnInst, Value) diff --git a/lib/Analysis/CaptureTracking.cpp b/lib/Analysis/CaptureTracking.cpp index fc23aa53d7c..de5ec29c857 100644 --- a/lib/Analysis/CaptureTracking.cpp +++ b/lib/Analysis/CaptureTracking.cpp @@ -80,11 +80,12 @@ namespace { if (BB == BeforeHere->getParent()) { // 'I' dominates 'BeforeHere' => not safe to prune. // - // The value defined by an invoke dominates an instruction only if it - // dominates every instruction in UseBB. A PHI is dominated only if - // the instruction dominates every possible use in the UseBB. Since + // The value defined by an invoke/catchpad dominates an instruction only + // if it dominates every instruction in UseBB. A PHI is dominated only + // if the instruction dominates every possible use in the UseBB. Since // UseBB == BB, avoid pruning. - if (isa(BeforeHere) || isa(I) || I == BeforeHere) + if (isa(BeforeHere) || isa(BeforeHere) || + isa(I) || I == BeforeHere) return false; if (!OrderedBB->dominates(BeforeHere, I)) return false; diff --git a/lib/Analysis/InstructionSimplify.cpp b/lib/Analysis/InstructionSimplify.cpp index fa42b48b6cd..f9d4524f012 100644 --- a/lib/Analysis/InstructionSimplify.cpp +++ b/lib/Analysis/InstructionSimplify.cpp @@ -123,9 +123,9 @@ static bool ValueDominatesPHI(Value *V, PHINode *P, const DominatorTree *DT) { } // Otherwise, if the instruction is in the entry block, and is not an invoke, - // then it obviously dominates all phi nodes. + // and is not a catchpad, then it obviously dominates all phi nodes. if (I->getParent() == &I->getParent()->getParent()->getEntryBlock() && - !isa(I)) + !isa(I) && !isa(I)) return true; return false; diff --git a/lib/Analysis/ScalarEvolutionExpander.cpp b/lib/Analysis/ScalarEvolutionExpander.cpp index 42069d29403..7789423cf6c 100644 --- a/lib/Analysis/ScalarEvolutionExpander.cpp +++ b/lib/Analysis/ScalarEvolutionExpander.cpp @@ -146,6 +146,8 @@ Value *SCEVExpander::InsertNoopCastOfTo(Value *V, Type *Ty) { BasicBlock::iterator IP = I; ++IP; if (InvokeInst *II = dyn_cast(I)) IP = II->getNormalDest()->begin(); + if (CatchPadInst *CPI = dyn_cast(I)) + IP = CPI->getNormalDest()->begin(); while (isa(IP) || isa(IP)) ++IP; return ReuseOrCreateCast(I, Ty, Op, IP); diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index f098babb9f4..83bc33d0e6c 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -5027,13 +5027,24 @@ bool LLParser::ParseCleanupRet(Instruction *&Inst, PerFunctionState &PFS) { } /// ParseCatchRet -/// ::= 'catchret' TypeAndValue +/// ::= 'catchret' ('void' | TypeAndValue) 'to' TypeAndValue bool LLParser::ParseCatchRet(Instruction *&Inst, PerFunctionState &PFS) { + Type *RetTy = nullptr; + Value *RetVal = nullptr; + + if (ParseType(RetTy, /*AllowVoid=*/true)) + return true; + + if (!RetTy->isVoidTy()) + if (ParseValue(RetTy, RetVal, PFS)) + return true; + BasicBlock *BB; - if (ParseTypeAndBasicBlock(BB, PFS)) + if (ParseToken(lltok::kw_to, "expected 'to' in catchret") || + ParseTypeAndBasicBlock(BB, PFS)) return true; - Inst = CatchReturnInst::Create(BB); + Inst = CatchReturnInst::Create(BB, RetVal); return false; } diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 06935f7ab1b..b379e59236d 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -3847,12 +3847,18 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) { break; } case bitc::FUNC_CODE_INST_CATCHRET: { // CATCHRET: [bb#] - if (Record.size() != 1) + if (Record.size() != 1 && Record.size() != 3) return error("Invalid record"); - BasicBlock *BB = getBasicBlock(Record[0]); + unsigned Idx = 0; + BasicBlock *BB = getBasicBlock(Record[Idx++]); if (!BB) return error("Invalid record"); - I = CatchReturnInst::Create(BB); + Value *RetVal = nullptr; + if (Record.size() == 3 && + getValueTypePair(Record, Idx, NextValueNo, RetVal)) + return error("Invalid record"); + + I = CatchReturnInst::Create(BB, RetVal); InstructionList.push_back(I); break; } diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 03650ec37b7..87b02e3dca4 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -1867,6 +1867,8 @@ static void WriteInstruction(const Instruction &I, unsigned InstID, Code = bitc::FUNC_CODE_INST_CATCHRET; const auto &CRI = cast(I); Vals.push_back(VE.getValueID(CRI.getSuccessor())); + if (CRI.hasReturnValue()) + PushValueAndType(CRI.getReturnValue(), InstID, Vals, VE); break; } case Instruction::CatchPad: { diff --git a/lib/IR/AsmWriter.cpp b/lib/IR/AsmWriter.cpp index 9ce384825f4..5ec111869f0 100644 --- a/lib/IR/AsmWriter.cpp +++ b/lib/IR/AsmWriter.cpp @@ -2900,6 +2900,16 @@ void AssemblyWriter::printInstruction(const Instruction &I) { Out << "]"; } else if (isa(I) && !Operand) { Out << " void"; + } else if (const auto *CRI = dyn_cast(&I)) { + if (CRI->hasReturnValue()) { + Out << ' '; + writeOperand(CRI->getReturnValue(), /*PrintType=*/true); + } else { + Out << " void"; + } + + Out << " to "; + writeOperand(CRI->getSuccessor(), /*PrintType=*/true); } else if (const auto *CRI = dyn_cast(&I)) { if (CRI->hasReturnValue()) { Out << ' '; diff --git a/lib/IR/Dominators.cpp b/lib/IR/Dominators.cpp index b6a8bbcbe5f..775bd92f2ed 100644 --- a/lib/IR/Dominators.cpp +++ b/lib/IR/Dominators.cpp @@ -91,11 +91,11 @@ bool DominatorTree::dominates(const Instruction *Def, if (Def == User) return false; - // The value defined by an invoke dominates an instruction only if + // The value defined by an invoke/catchpad dominates an instruction only if // it dominates every instruction in UseBB. // A PHI is dominated only if the instruction dominates every possible use // in the UseBB. - if (isa(Def) || isa(User)) + if (isa(Def) || isa(Def) || isa(User)) return dominates(Def, UseBB); if (DefBB != UseBB) @@ -126,15 +126,20 @@ bool DominatorTree::dominates(const Instruction *Def, if (DefBB == UseBB) return false; - const InvokeInst *II = dyn_cast(Def); - if (!II) - return dominates(DefBB, UseBB); + // Invoke/CatchPad results are only usable in the normal destination, not in + // the exceptional destination. + if (const auto *II = dyn_cast(Def)) { + BasicBlock *NormalDest = II->getNormalDest(); + BasicBlockEdge E(DefBB, NormalDest); + return dominates(E, UseBB); + } + if (const auto *CPI = dyn_cast(Def)) { + BasicBlock *NormalDest = CPI->getNormalDest(); + BasicBlockEdge E(DefBB, NormalDest); + return dominates(E, UseBB); + } - // Invoke results are only usable in the normal destination, not in the - // exceptional destination. - BasicBlock *NormalDest = II->getNormalDest(); - BasicBlockEdge E(DefBB, NormalDest); - return dominates(E, UseBB); + return dominates(DefBB, UseBB); } bool DominatorTree::dominates(const BasicBlockEdge &BBE, @@ -232,7 +237,7 @@ bool DominatorTree::dominates(const Instruction *Def, const Use &U) const { if (!isReachableFromEntry(DefBB)) return false; - // Invoke instructions define their return values on the edges + // Invoke/CatchPad instructions define their return values on the edges // to their normal successors, so we have to handle them specially. // Among other things, this means they don't dominate anything in // their own block, except possibly a phi, so we don't need to @@ -242,6 +247,11 @@ bool DominatorTree::dominates(const Instruction *Def, const Use &U) const { BasicBlockEdge E(DefBB, NormalDest); return dominates(E, U); } + if (const auto *CPI = dyn_cast(Def)) { + BasicBlock *NormalDest = CPI->getNormalDest(); + BasicBlockEdge E(DefBB, NormalDest); + return dominates(E, U); + } // If the def and use are in different blocks, do a simple CFG dominator // tree query. diff --git a/lib/IR/Instructions.cpp b/lib/IR/Instructions.cpp index 79fb30e4734..46c799ece2c 100644 --- a/lib/IR/Instructions.cpp +++ b/lib/IR/Instructions.cpp @@ -797,27 +797,38 @@ void CatchEndPadInst::setSuccessorV(unsigned Idx, BasicBlock *B) { //===----------------------------------------------------------------------===// // CatchReturnInst Implementation //===----------------------------------------------------------------------===// +void CatchReturnInst::init(BasicBlock *BB, Value *RetVal) { + Op<-1>() = BB; + if (RetVal) + Op<-2>() = RetVal; +} CatchReturnInst::CatchReturnInst(const CatchReturnInst &CRI) : TerminatorInst(Type::getVoidTy(CRI.getContext()), Instruction::CatchRet, OperandTraits::op_end(this) - CRI.getNumOperands(), CRI.getNumOperands()) { - Op<0>() = CRI.Op<0>(); + Op<-1>() = CRI.Op<-1>(); + if (CRI.getNumOperands() != 1) { + assert(CRI.getNumOperands() == 2); + Op<-2>() = CRI.Op<-2>(); + } } -CatchReturnInst::CatchReturnInst(BasicBlock *BB, Instruction *InsertBefore) +CatchReturnInst::CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values, + Instruction *InsertBefore) : TerminatorInst(Type::getVoidTy(BB->getContext()), Instruction::CatchRet, - OperandTraits::op_begin(this), 1, - InsertBefore) { - Op<0>() = BB; + OperandTraits::op_end(this) - Values, + Values, InsertBefore) { + init(BB, RetVal); } -CatchReturnInst::CatchReturnInst(BasicBlock *BB, BasicBlock *InsertAtEnd) +CatchReturnInst::CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values, + BasicBlock *InsertAtEnd) : TerminatorInst(Type::getVoidTy(BB->getContext()), Instruction::CatchRet, - OperandTraits::op_begin(this), 1, - InsertAtEnd) { - Op<0>() = BB; + OperandTraits::op_end(this) - Values, + Values, InsertAtEnd) { + init(BB, RetVal); } BasicBlock *CatchReturnInst::getSuccessorV(unsigned Idx) const { @@ -3924,7 +3935,7 @@ CatchEndPadInst *CatchEndPadInst::cloneImpl() const { } CatchReturnInst *CatchReturnInst::cloneImpl() const { - return new (1) CatchReturnInst(*this); + return new (getNumOperands()) CatchReturnInst(*this); } CatchPadInst *CatchPadInst::cloneImpl() const { diff --git a/lib/Transforms/Scalar/Reassociate.cpp b/lib/Transforms/Scalar/Reassociate.cpp index 1626548541f..21245864544 100644 --- a/lib/Transforms/Scalar/Reassociate.cpp +++ b/lib/Transforms/Scalar/Reassociate.cpp @@ -951,6 +951,8 @@ static Value *NegateValue(Value *V, Instruction *BI) { if (Instruction *InstInput = dyn_cast(V)) { if (InvokeInst *II = dyn_cast(InstInput)) { InsertPt = II->getNormalDest()->begin(); + } else if (auto *CPI = dyn_cast(InstInput)) { + InsertPt = CPI->getNormalDest()->begin(); } else { InsertPt = InstInput; ++InsertPt; diff --git a/lib/Transforms/Utils/CodeExtractor.cpp b/lib/Transforms/Utils/CodeExtractor.cpp index 896cff1bae9..e76a9d4ba2d 100644 --- a/lib/Transforms/Utils/CodeExtractor.cpp +++ b/lib/Transforms/Utils/CodeExtractor.cpp @@ -560,14 +560,20 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer, // Restore values just before we exit Function::arg_iterator OAI = OutputArgBegin; for (unsigned out = 0, e = outputs.size(); out != e; ++out) { - // For an invoke, the normal destination is the only one that is - // dominated by the result of the invocation + // For an invoke/catchpad, the normal destination is the only one + // that is dominated by the result of the invocation BasicBlock *DefBlock = cast(outputs[out])->getParent(); bool DominatesDef = true; - if (InvokeInst *Invoke = dyn_cast(outputs[out])) { - DefBlock = Invoke->getNormalDest(); + BasicBlock *NormalDest = nullptr; + if (auto *Invoke = dyn_cast(outputs[out])) + NormalDest = Invoke->getNormalDest(); + if (auto *CatchPad = dyn_cast(outputs[out])) + NormalDest = CatchPad->getNormalDest(); + + if (NormalDest) { + DefBlock = NormalDest; // Make sure we are looking at the original successor block, not // at a newly inserted exit block, which won't be in the dominator diff --git a/lib/Transforms/Utils/LCSSA.cpp b/lib/Transforms/Utils/LCSSA.cpp index 9d40b6989d6..d45944d47cf 100644 --- a/lib/Transforms/Utils/LCSSA.cpp +++ b/lib/Transforms/Utils/LCSSA.cpp @@ -82,14 +82,15 @@ static bool processInstruction(Loop &L, Instruction &Inst, DominatorTree &DT, ++NumLCSSA; // We are applying the transformation - // Invoke instructions are special in that their result value is not available - // along their unwind edge. The code below tests to see whether DomBB - // dominates - // the value, so adjust DomBB to the normal destination block, which is - // effectively where the value is first usable. + // Invoke/CatchPad instructions are special in that their result value is not + // available along their unwind edge. The code below tests to see whether + // DomBB dominates the value, so adjust DomBB to the normal destination block, + // which is effectively where the value is first usable. BasicBlock *DomBB = Inst.getParent(); if (InvokeInst *Inv = dyn_cast(&Inst)) DomBB = Inv->getNormalDest(); + if (auto *CPI = dyn_cast(&Inst)) + DomBB = CPI->getNormalDest(); DomTreeNode *DomNode = DT.getNode(DomBB); diff --git a/test/CodeGen/WinEH/wineh-demotion.ll b/test/CodeGen/WinEH/wineh-demotion.ll index eb342bc388c..9f49e1379a2 100644 --- a/test/CodeGen/WinEH/wineh-demotion.ll +++ b/test/CodeGen/WinEH/wineh-demotion.ll @@ -43,7 +43,7 @@ catch: ; CHECK: [[Reload:%[^ ]+]] = load i32, i32* [[Slot]] ; CHECK-NEXT: call void @h(i32 [[Reload]]) call void @h(i32 %phi) - catchret label %exit + catchret void to label %exit catchend: catchendpad unwind to caller @@ -89,7 +89,7 @@ catch.inner: to label %catchret.inner unwind label %merge.outer catchret.inner: - catchret label %exit + catchret void to label %exit catchend.inner: catchendpad unwind label %merge.outer @@ -109,10 +109,10 @@ catch.outer: ; CHECK: catch.outer: ; CHECK-DAG: load i32, i32* [[Slot1]] ; CHECK-DAG: load i32, i32* [[Slot2]] - ; CHECK: catchret label + ; CHECK: catchret void to label call void @h(i32 %x) call void @h(i32 %y) - catchret label %exit + catchret void to label %exit exit: ret void @@ -152,7 +152,7 @@ merge: ; CHECK: %phi = phi i32 [ [[ReloadX]], %left ] %phi = phi i32 [ %x, %left ], [ 42, %right ] call void @h(i32 %phi) - catchret label %exit + catchret void to label %exit catchend: catchendpad unwind to caller @@ -192,7 +192,7 @@ catchpad.inner: %phi.inner = phi i32 [ %l, %left ], [ %r, %right ] catchpad void [] to label %catch.inner unwind label %catchend.inner catch.inner: - catchret label %join + catchret void to label %join catchend.inner: catchendpad unwind label %catchpad.outer join: @@ -213,7 +213,7 @@ catch.outer: ; CHECK: [[Reload:%[^ ]+]] = load i32, i32* [[Slot]] ; CHECK: call void @h(i32 [[Reload]]) call void @h(i32 %phi.outer) - catchret label %exit + catchret void to label %exit catchend.outer: catchendpad unwind to caller exit: @@ -286,7 +286,7 @@ catch: ; CHECK: [[CatchReload:%[^ ]+]] = load i32, i32* [[CatchSlot]] ; CHECK: call void @h(i32 [[CatchReload]] call void @h(i32 %phi.catch) - catchret label %exit + catchret void to label %exit catchend: catchendpad unwind to caller diff --git a/test/Feature/exception.ll b/test/Feature/exception.ll index dbbe7e41edb..e2635c2de42 100644 --- a/test/Feature/exception.ll +++ b/test/Feature/exception.ll @@ -63,7 +63,7 @@ define void @catchret() personality i32 (...)* @__gxx_personality_v0 { entry: br label %bb bb: - catchret label %bb + catchret void to label %bb } define i8 @catchpad() personality i32 (...)* @__gxx_personality_v0 { @@ -72,11 +72,16 @@ entry: try.cont: invoke void @_Z3quxv() optsize - to label %bb unwind label %bb2 + to label %exit unwind label %bb2 bb: + catchret token %cbv to label %exit + +exit: ret i8 0 bb2: - %cbv = catchpad i8 [i7 4] to label %bb unwind label %bb2 + %cbv = catchpad token [i7 4] to label %bb unwind label %bb3 +bb3: + catchendpad unwind to caller } define void @terminatepad0() personality i32 (...)* @__gxx_personality_v0 { -- 2.34.1