Address Joseph's review comments.

llvm-svn: 241890

docs/LangRef.rst

@@ -4730,6 +4730,7 @@ The terminator instructions are: ':ref:`ret <i_ret>`',
 ':ref:`resume <i_resume>`', ':ref:`catchblock <i_catchblock>`',
 ':ref:`catchendblock <i_catchendblock>`',
 ':ref:`catchret <i_catchret>`',
+':ref:`cleanupret <i_cleanupret>`',
 ':ref:`terminateblock <i_terminateblock>`',
 and ':ref:`unreachable <i_unreachable>`'.
 
@@ -5143,6 +5144,8 @@ The ``catchblock`` instruction has several restrictions:
    an exceptional instruction.
 -  A catch block must have a '``catchblock``' instruction as its
    first non-PHI instruction.
+-  A catch block's ``exception`` edge must refer to a catch block or a
+   catch-end block.
 -  There can be only one '``catchblock``' instruction within the
    catch block.
 -  A basic block that is not a catch block may not include a
@@ -5252,15 +5255,17 @@ Semantics:
 
 The '``catchret``' instruction ends the existing (in-flight) exception
 whose unwinding was interrupted with a
-:ref:`catchblock <i_catchblock>` instruction and transfers control to
-``normal``.
+:ref:`catchblock <i_catchblock>` instruction.
+The :ref:`personality function <personalityfn>` gets a chance to execute
+arbitrary code to, for example, run a C++ destructor.
+Control then transfers to ``normal``.
 
 Example:
 """"""""
 
 .. code-block:: llvm
 
-      catchret unwind label %continue
+      catchret label %continue
 
 .. _i_cleanupret:
 
@@ -5293,8 +5298,8 @@ Semantics:
 """"""""""
 
 The '``cleanupret``' instruction indicates to the
-:ref:`personality function <personalityfn>` that the
-:ref:`cleanupblock <i_cleanupblock>` it transfered control to has ended.
+:ref:`personality function <personalityfn>` that one
+:ref:`cleanupblock <i_cleanupblock>` it transferred control to has ended.
 It transfers control to ``continue`` or unwinds out of the function.
 
 Example:
@@ -5327,7 +5332,7 @@ is a terminate block --- one where a personality routine may decide to
 terminate the program.
 The ``args`` correspond to whatever information the personality
 routine requires to know if this is an appropriate place to terminate the
-program.  Control is tranfered to the ``exception`` label if the
+program.  Control is transferred to the ``exception`` label if the
 personality routine decides not to terminate the program for the
 in-flight exception.
 
@@ -8339,8 +8344,7 @@ transfer control to run cleanup actions.
 The ``args`` correspond to whatever additional
 information the :ref:`personality function <personalityfn>` requires to
 execute the cleanup.
-:ref:`personality function <personalityfn>` upon re-entry to the
-function. The ``resultval`` has the type ``resultty``.
+The ``resultval`` has the type ``resultty``.
 
 Arguments:
 """"""""""

include/llvm/IR/Instructions.h

@@ -3805,12 +3805,12 @@ public:
   void setUnwindDest(BasicBlock *B) { Op<-1>() = reinterpret_cast<Value *>(B); }
 
   BasicBlock *getSuccessor(unsigned i) const {
-    assert(i < 2 && "Successor # out of range for invoke!");
+    assert(i < 2 && "Successor # out of range for catchblock!");
     return i == 0 ? getNormalDest() : getUnwindDest();
   }
 
   void setSuccessor(unsigned idx, BasicBlock *NewSucc) {
-    assert(idx < 2 && "Successor # out of range for invoke!");
+    assert(idx < 2 && "Successor # out of range for catchblock!");
     *(&Op<-2>() + idx) = reinterpret_cast<Value *>(NewSucc);
   }
 

lib/AsmParser/LLParser.cpp

@@ -4994,7 +4994,7 @@ bool LLParser::ParseTerminateBlock(Instruction *&Inst, PerFunctionState &PFS) {
   BasicBlock *UnwindBB = nullptr;
   if (Lex.getKind() == lltok::kw_to) {
     Lex.Lex();
-    if (ParseToken(lltok::kw_caller, "expected 'caller' in cleanupret"))
+    if (ParseToken(lltok::kw_caller, "expected 'caller' in terminateblock"))
       return true;
   } else {
     if (ParseTypeAndBasicBlock(UnwindBB, PFS)) {
@@ -5022,7 +5022,7 @@ bool LLParser::ParseCleanupBlock(Instruction *&Inst, PerFunctionState &PFS) {
 /// ParseCatchEndBlock
 ///   ::= 'catchendblock' unwind ('to' 'caller' | TypeAndValue)
 bool LLParser::ParseCatchEndBlock(Instruction *&Inst, PerFunctionState &PFS) {
-  if (ParseToken(lltok::kw_unwind, "expected 'unwind' in cleanupret"))
+  if (ParseToken(lltok::kw_unwind, "expected 'unwind' in catchendblock"))
     return true;
 
   BasicBlock *UnwindBB = nullptr;

lib/IR/Instruction.cpp

@@ -469,6 +469,8 @@ bool Instruction::mayThrow() const {
     return CRI->unwindsToCaller();
   if (const auto *CEBI = dyn_cast<CatchEndBlockInst>(this))
     return CEBI->unwindsToCaller();
+  if (const auto *TBI = dyn_cast<TerminateBlockInst>(this))
+    return TBI->unwindsToCaller();
   return isa<ResumeInst>(this);
 }
 

lib/IR/Verifier.cpp

@@ -184,6 +184,9 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   /// \brief Track unresolved string-based type references.
   SmallDenseMap<const MDString *, const MDNode *, 32> UnresolvedTypeRefs;
 
+  /// \brief The result value from the personality function.
+  Type *PersonalityFnResultTy;
+
   /// \brief Whether we've seen a call to @llvm.localescape in this function
   /// already.
   bool SawFrameEscape;
@@ -194,7 +197,8 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
 
 public:
   explicit Verifier(raw_ostream &OS)
-      : VerifierSupport(OS), Context(nullptr), SawFrameEscape(false) {}
+      : VerifierSupport(OS), Context(nullptr), PersonalityFnResultTy(nullptr),
+        SawFrameEscape(false) {}
 
   bool verify(const Function &F) {
     M = F.getParent();
@@ -228,6 +232,7 @@ public:
     // FIXME: We strip const here because the inst visitor strips const.
     visit(const_cast<Function &>(F));
     InstsInThisBlock.clear();
+    PersonalityFnResultTy = nullptr;
     SawFrameEscape = false;
 
     return !Broken;
@@ -383,6 +388,7 @@ private:
   void visitCatchBlockInst(CatchBlockInst &CBI);
   void visitCatchEndBlockInst(CatchEndBlockInst &CEBI);
   void visitCleanupBlockInst(CleanupBlockInst &CBI);
+  void visitCleanupReturnInst(CleanupReturnInst &CRI);
   void visitTerminateBlockInst(TerminateBlockInst &TBI);
 
   void VerifyCallSite(CallSite CS);
@@ -2799,6 +2805,14 @@ void Verifier::visitLandingPadInst(LandingPadInst &LPI) {
            &LPI);
   }
 
+  if (!PersonalityFnResultTy)
+    PersonalityFnResultTy = LPI.getType();
+  else
+    Assert(PersonalityFnResultTy == LPI.getType(),
+           "The personality routine should have a consistent result type "
+           "inside a function.",
+           &LPI);
+
   Function *F = LPI.getParent()->getParent();
   Assert(F->hasPersonalityFn(),
          "LandingPadInst needs to be in a function with a personality.", &LPI);
@@ -2827,6 +2841,14 @@ void Verifier::visitLandingPadInst(LandingPadInst &LPI) {
 void Verifier::visitCatchBlockInst(CatchBlockInst &CBI) {
   BasicBlock *BB = CBI.getParent();
 
+  if (!PersonalityFnResultTy)
+    PersonalityFnResultTy = CBI.getType();
+  else
+    Assert(PersonalityFnResultTy == CBI.getType(),
+           "The personality routine should have a consistent result type "
+           "inside a function.",
+           &CBI);
+
   Function *F = BB->getParent();
   Assert(F->hasPersonalityFn(),
          "CatchBlockInst needs to be in a function with a personality.", &CBI);
@@ -2837,6 +2859,12 @@ void Verifier::visitCatchBlockInst(CatchBlockInst &CBI) {
          "CatchBlockInst not the first non-PHI instruction in the block.",
          &CBI);
 
+  BasicBlock *UnwindDest = CBI.getUnwindDest();
+  Instruction *I = UnwindDest->getFirstNonPHI();
+  Assert(I->isEHBlock() && !isa<LandingPadInst>(I),
+         "CatchBlockInst must unwind to an EH block which is not a landingpad.",
+         &CBI);
+
   visitTerminatorInst(CBI);
 }
 
@@ -2854,12 +2882,37 @@ void Verifier::visitCatchEndBlockInst(CatchEndBlockInst &CEBI) {
          "CatchEndBlockInst not the first non-PHI instruction in the block.",
          &CEBI);
 
+  unsigned CatchBlocksSeen = 0;
+  for (BasicBlock *PredBB : predecessors(BB))
+    if (isa<CatchBlockInst>(PredBB->getTerminator()))
+      ++CatchBlocksSeen;
+
+  Assert(CatchBlocksSeen <= 1, "CatchEndBlockInst must have no more than one "
+                               "CatchBlockInst predecessor.",
+         &CEBI);
+
+  if (BasicBlock *UnwindDest = CEBI.getUnwindDest()) {
+    Instruction *I = UnwindDest->getFirstNonPHI();
+    Assert(
+        I->isEHBlock() && !isa<LandingPadInst>(I),
+        "CatchEndBlock must unwind to an EH block which is not a landingpad.",
+        &CEBI);
+  }
+
   visitTerminatorInst(CEBI);
 }
 
 void Verifier::visitCleanupBlockInst(CleanupBlockInst &CBI) {
   BasicBlock *BB = CBI.getParent();
 
+  if (!PersonalityFnResultTy)
+    PersonalityFnResultTy = CBI.getType();
+  else
+    Assert(PersonalityFnResultTy == CBI.getType(),
+           "The personality routine should have a consistent result type "
+           "inside a function.",
+           &CBI);
+
   Function *F = BB->getParent();
   Assert(F->hasPersonalityFn(),
          "CleanupBlockInst needs to be in a function with a personality.", &CBI);
@@ -2873,6 +2926,18 @@ void Verifier::visitCleanupBlockInst(CleanupBlockInst &CBI) {
   visitInstruction(CBI);
 }
 
+void Verifier::visitCleanupReturnInst(CleanupReturnInst &CRI) {
+  if (BasicBlock *UnwindDest = CRI.getUnwindDest()) {
+    Instruction *I = UnwindDest->getFirstNonPHI();
+    Assert(I->isEHBlock() && !isa<LandingPadInst>(I),
+           "CleanupReturnInst must unwind to an EH block which is not a "
+           "landingpad.",
+           &CRI);
+  }
+
+  visitTerminatorInst(CRI);
+}
+
 void Verifier::visitTerminateBlockInst(TerminateBlockInst &TBI) {
   BasicBlock *BB = TBI.getParent();
 
@@ -2887,6 +2952,14 @@ void Verifier::visitTerminateBlockInst(TerminateBlockInst &TBI) {
          "TerminateBlockInst not the first non-PHI instruction in the block.",
          &TBI);
 
+  if (BasicBlock *UnwindDest = TBI.getUnwindDest()) {
+    Instruction *I = UnwindDest->getFirstNonPHI();
+    Assert(I->isEHBlock() && !isa<LandingPadInst>(I),
+           "TerminateBlockInst must unwind to an EH block which is not a "
+           "landingpad.",
+           &TBI);
+  }
+
   visitTerminatorInst(TBI);
 }