[WinEH] Require token linkage in EH pad/ret signatures

Summary:
WinEHPrepare is going to require that cleanuppad and catchpad produce values
of token type which are consumed by any cleanupret or catchret exiting the
pad.  This change updates the signatures of those operators to require/enforce
that the type produced by the pads is token type and that the rets have an
appropriate argument.

The catchpad argument of a `CatchReturnInst` must be a `CatchPadInst` (and
similarly for `CleanupReturnInst`/`CleanupPadInst`).  To accommodate that
restriction, this change adds a notion of an operator constraint to both
LLParser and BitcodeReader, allowing appropriate sentinels to be constructed
for forward references and appropriate error messages to be emitted for
illegal inputs.

Also add a verifier rule (noted in LangRef) that a catchpad with a catchpad
predecessor must have no other predecessors; this ensures that WinEHPrepare
will see the expected linear relationship between sibling catches on the
same try.

Lastly, remove some superfluous/vestigial casts from instruction operand
setters operating on BasicBlocks.

Reviewers: rnk, majnemer

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D12108

llvm-svn: 245797

4 files changed, 68 insertions, 105 deletions

diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 5ec111869f0..ae9ab8d6187 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2860,9 +2860,6 @@ void AssemblyWriter::printInstruction(const Instruction &I) {
       writeOperand(LPI->getClause(i), true);
     }
   } else if (const auto *CPI = dyn_cast<CatchPadInst>(&I)) {
-    Out << ' ';
-    TypePrinter.print(I.getType(), Out);
-
     Out << " [";
     for (unsigned Op = 0, NumOps = CPI->getNumArgOperands(); Op < NumOps;
          ++Op) {
@@ -2888,9 +2885,6 @@ void AssemblyWriter::printInstruction(const Instruction &I) {
     else
       Out << "to caller";
   } else if (const auto *CPI = dyn_cast<CleanupPadInst>(&I)) {
-    Out << ' ';
-    TypePrinter.print(I.getType(), Out);
-
     Out << " [";
     for (unsigned Op = 0, NumOps = CPI->getNumOperands(); Op < NumOps; ++Op) {
       if (Op > 0)
@@ -2901,22 +2895,14 @@ void AssemblyWriter::printInstruction(const Instruction &I) {
   } else if (isa<ReturnInst>(I) && !Operand) {
     Out << " void";
   } else if (const auto *CRI = dyn_cast<CatchReturnInst>(&I)) {
-    if (CRI->hasReturnValue()) {
-      Out << ' ';
-      writeOperand(CRI->getReturnValue(), /*PrintType=*/true);
-    } else {
-      Out << " void";
-    }
+    Out << ' ';
+    writeOperand(CRI->getCatchPad(), /*PrintType=*/false);
 
     Out << " to ";
     writeOperand(CRI->getSuccessor(), /*PrintType=*/true);
   } else if (const auto *CRI = dyn_cast<CleanupReturnInst>(&I)) {
-    if (CRI->hasReturnValue()) {
-      Out << ' ';
-      writeOperand(CRI->getReturnValue(), /*PrintType=*/true);
-    } else {
-      Out << " void";
-    }
+    Out << ' ';
+    writeOperand(CRI->getCleanupPad(), /*PrintType=*/false);
 
     Out << " unwind ";
     if (CRI->hasUnwindDest())
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 85173d96562..0dc8633ac2a 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -261,7 +261,7 @@ const char *Instruction::getOpcodeName(unsigned OpCode) {
   case ExtractValue:   return "extractvalue";
   case InsertValue:    return "insertvalue";
   case LandingPad:     return "landingpad";
-  case CleanupPad:   return "cleanuppad";
+  case CleanupPad:     return "cleanuppad";
 
   default: return "<Invalid operator> ";
   }
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 46c799ece2c..e79fa415ffb 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -684,51 +684,39 @@ CleanupReturnInst::CleanupReturnInst(const CleanupReturnInst &CRI)
                      CRI.getNumOperands()) {
   SubclassOptionalData = CRI.SubclassOptionalData;
   setInstructionSubclassData(CRI.getSubclassDataFromInstruction());
-  if (Value *RetVal = CRI.getReturnValue())
-    setReturnValue(RetVal);
-  if (BasicBlock *UnwindDest = CRI.getUnwindDest())
-    setUnwindDest(UnwindDest);
+  Op<-1>() = CRI.Op<-1>();
+  if (CRI.hasUnwindDest())
+    Op<-2>() = CRI.Op<-2>();
 }
 
-void CleanupReturnInst::init(Value *RetVal, BasicBlock *UnwindBB) {
+void CleanupReturnInst::init(CleanupPadInst *CleanupPad, BasicBlock *UnwindBB) {
   SubclassOptionalData = 0;
   if (UnwindBB)
     setInstructionSubclassData(getSubclassDataFromInstruction() | 1);
-  if (RetVal)
-    setInstructionSubclassData(getSubclassDataFromInstruction() | 2);
 
+  Op<-1>() = CleanupPad;
   if (UnwindBB)
-    setUnwindDest(UnwindBB);
-  if (RetVal)
-    setReturnValue(RetVal);
+    Op<-2>() = UnwindBB;
 }
 
-CleanupReturnInst::CleanupReturnInst(LLVMContext &C, Value *RetVal,
+CleanupReturnInst::CleanupReturnInst(CleanupPadInst *CleanupPad,
                                      BasicBlock *UnwindBB, unsigned Values,
                                      Instruction *InsertBefore)
-    : TerminatorInst(Type::getVoidTy(C), Instruction::CleanupRet,
+    : TerminatorInst(Type::getVoidTy(CleanupPad->getContext()),
+                     Instruction::CleanupRet,
                      OperandTraits<CleanupReturnInst>::op_end(this) - Values,
                      Values, InsertBefore) {
-  init(RetVal, UnwindBB);
+  init(CleanupPad, UnwindBB);
 }
 
-CleanupReturnInst::CleanupReturnInst(LLVMContext &C, Value *RetVal,
+CleanupReturnInst::CleanupReturnInst(CleanupPadInst *CleanupPad,
                                      BasicBlock *UnwindBB, unsigned Values,
                                      BasicBlock *InsertAtEnd)
-    : TerminatorInst(Type::getVoidTy(C), Instruction::CleanupRet,
+    : TerminatorInst(Type::getVoidTy(CleanupPad->getContext()),
+                     Instruction::CleanupRet,
                      OperandTraits<CleanupReturnInst>::op_end(this) - Values,
                      Values, InsertAtEnd) {
-  init(RetVal, UnwindBB);
-}
-
-BasicBlock *CleanupReturnInst::getUnwindDest() const {
-  if (hasUnwindDest())
-    return cast<BasicBlock>(getOperand(getUnwindLabelOpIdx()));
-  return nullptr;
-}
-void CleanupReturnInst::setUnwindDest(BasicBlock *NewDest) {
-  assert(NewDest);
-  setOperand(getUnwindLabelOpIdx(), NewDest);
+  init(CleanupPad, UnwindBB);
 }
 
 BasicBlock *CleanupReturnInst::getSuccessorV(unsigned Idx) const {
@@ -797,38 +785,32 @@ void CatchEndPadInst::setSuccessorV(unsigned Idx, BasicBlock *B) {
 //===----------------------------------------------------------------------===//
 //                        CatchReturnInst Implementation
 //===----------------------------------------------------------------------===//
-void CatchReturnInst::init(BasicBlock *BB, Value *RetVal) {
-  Op<-1>() = BB;
-  if (RetVal)
-    Op<-2>() = RetVal;
+void CatchReturnInst::init(CatchPadInst *CatchPad, BasicBlock *BB) {
+  Op<0>() = CatchPad;
+  Op<1>() = BB;
 }
 
 CatchReturnInst::CatchReturnInst(const CatchReturnInst &CRI)
     : TerminatorInst(Type::getVoidTy(CRI.getContext()), Instruction::CatchRet,
-                     OperandTraits<CatchReturnInst>::op_end(this) -
-                         CRI.getNumOperands(),
-                     CRI.getNumOperands()) {
-  Op<-1>() = CRI.Op<-1>();
-  if (CRI.getNumOperands() != 1) {
-    assert(CRI.getNumOperands() == 2);
-    Op<-2>() = CRI.Op<-2>();
-  }
+                     OperandTraits<CatchReturnInst>::op_begin(this), 2) {
+  Op<0>() = CRI.Op<0>();
+  Op<1>() = CRI.Op<1>();
 }
 
-CatchReturnInst::CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values,
+CatchReturnInst::CatchReturnInst(CatchPadInst *CatchPad, BasicBlock *BB,
                                  Instruction *InsertBefore)
     : TerminatorInst(Type::getVoidTy(BB->getContext()), Instruction::CatchRet,
-                     OperandTraits<CatchReturnInst>::op_end(this) - Values,
-                     Values, InsertBefore) {
-  init(BB, RetVal);
+                     OperandTraits<CatchReturnInst>::op_begin(this), 2,
+                     InsertBefore) {
+  init(CatchPad, BB);
 }
 
-CatchReturnInst::CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values,
+CatchReturnInst::CatchReturnInst(CatchPadInst *CatchPad, BasicBlock *BB,
                                  BasicBlock *InsertAtEnd)
     : TerminatorInst(Type::getVoidTy(BB->getContext()), Instruction::CatchRet,
-                     OperandTraits<CatchReturnInst>::op_end(this) - Values,
-                     Values, InsertAtEnd) {
-  init(BB, RetVal);
+                     OperandTraits<CatchReturnInst>::op_begin(this), 2,
+                     InsertAtEnd) {
+  init(CatchPad, BB);
 }
 
 BasicBlock *CatchReturnInst::getSuccessorV(unsigned Idx) const {
@@ -863,21 +845,21 @@ CatchPadInst::CatchPadInst(const CatchPadInst &CPI)
   std::copy(CPI.op_begin(), CPI.op_end(), op_begin());
 }
 
-CatchPadInst::CatchPadInst(Type *RetTy, BasicBlock *IfNormal,
-                           BasicBlock *IfException, ArrayRef<Value *> Args,
-                           unsigned Values, const Twine &NameStr,
-                           Instruction *InsertBefore)
-    : TerminatorInst(RetTy, Instruction::CatchPad,
+CatchPadInst::CatchPadInst(BasicBlock *IfNormal, BasicBlock *IfException,
+                           ArrayRef<Value *> Args, unsigned Values,
+                           const Twine &NameStr, Instruction *InsertBefore)
+    : TerminatorInst(Type::getTokenTy(IfNormal->getContext()),
+                     Instruction::CatchPad,
                      OperandTraits<CatchPadInst>::op_end(this) - Values, Values,
                      InsertBefore) {
   init(IfNormal, IfException, Args, NameStr);
 }
 
-CatchPadInst::CatchPadInst(Type *RetTy, BasicBlock *IfNormal,
-                           BasicBlock *IfException, ArrayRef<Value *> Args,
-                           unsigned Values, const Twine &NameStr,
-                           BasicBlock *InsertAtEnd)
-    : TerminatorInst(RetTy, Instruction::CatchPad,
+CatchPadInst::CatchPadInst(BasicBlock *IfNormal, BasicBlock *IfException,
+                           ArrayRef<Value *> Args, unsigned Values,
+                           const Twine &NameStr, BasicBlock *InsertAtEnd)
+    : TerminatorInst(Type::getTokenTy(IfNormal->getContext()),
+                     Instruction::CatchPad,
                      OperandTraits<CatchPadInst>::op_end(this) - Values, Values,
                      InsertAtEnd) {
   init(IfNormal, IfException, Args, NameStr);
@@ -962,17 +944,17 @@ CleanupPadInst::CleanupPadInst(const CleanupPadInst &CPI)
   std::copy(CPI.op_begin(), CPI.op_end(), op_begin());
 }
 
-CleanupPadInst::CleanupPadInst(Type *RetTy, ArrayRef<Value *> Args,
+CleanupPadInst::CleanupPadInst(LLVMContext &C, ArrayRef<Value *> Args,
                                const Twine &NameStr, Instruction *InsertBefore)
-    : Instruction(RetTy, Instruction::CleanupPad,
+    : Instruction(Type::getTokenTy(C), Instruction::CleanupPad,
                   OperandTraits<CleanupPadInst>::op_end(this) - Args.size(),
                   Args.size(), InsertBefore) {
   init(Args, NameStr);
 }
 
-CleanupPadInst::CleanupPadInst(Type *RetTy, ArrayRef<Value *> Args,
+CleanupPadInst::CleanupPadInst(LLVMContext &C, ArrayRef<Value *> Args,
                                const Twine &NameStr, BasicBlock *InsertAtEnd)
-    : Instruction(RetTy, Instruction::CleanupPad,
+    : Instruction(Type::getTokenTy(C), Instruction::CleanupPad,
                   OperandTraits<CleanupPadInst>::op_end(this) - Args.size(),
                   Args.size(), InsertAtEnd) {
   init(Args, NameStr);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 813d06cf643..7e10e4e1cee 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -184,12 +184,6 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   /// \brief Track unresolved string-based type references.
   SmallDenseMap<const MDString *, const MDNode *, 32> UnresolvedTypeRefs;
 
-  /// \brief The result type for a catchpad.
-  Type *CatchPadResultTy;
-
-  /// \brief The result type for a cleanuppad.
-  Type *CleanupPadResultTy;
-
   /// \brief The result type for a landingpad.
   Type *LandingPadResultTy;
 
@@ -203,8 +197,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
 
 public:
   explicit Verifier(raw_ostream &OS)
-      : VerifierSupport(OS), Context(nullptr), CatchPadResultTy(nullptr),
-        CleanupPadResultTy(nullptr), LandingPadResultTy(nullptr),
+      : VerifierSupport(OS), Context(nullptr), LandingPadResultTy(nullptr),
         SawFrameEscape(false) {}
 
   bool verify(const Function &F) {
@@ -239,8 +232,6 @@ public:
     // FIXME: We strip const here because the inst visitor strips const.
     visit(const_cast<Function &>(F));
     InstsInThisBlock.clear();
-    CatchPadResultTy = nullptr;
-    CleanupPadResultTy = nullptr;
     LandingPadResultTy = nullptr;
     SawFrameEscape = false;
 
@@ -2877,14 +2868,6 @@ void Verifier::visitLandingPadInst(LandingPadInst &LPI) {
 void Verifier::visitCatchPadInst(CatchPadInst &CPI) {
   visitEHPadPredecessors(CPI);
 
-  if (!CatchPadResultTy)
-    CatchPadResultTy = CPI.getType();
-  else
-    Assert(CatchPadResultTy == CPI.getType(),
-           "The catchpad instruction should have a consistent result type "
-           "inside a function.",
-           &CPI);
-
   BasicBlock *BB = CPI.getParent();
   Function *F = BB->getParent();
   Assert(F->hasPersonalityFn(),
@@ -2896,6 +2879,14 @@ void Verifier::visitCatchPadInst(CatchPadInst &CPI) {
          "CatchPadInst not the first non-PHI instruction in the block.",
          &CPI);
 
+  if (!BB->getSinglePredecessor())
+    for (BasicBlock *PredBB : predecessors(BB)) {
+      Assert(!isa<CatchPadInst>(PredBB->getTerminator()),
+             "CatchPadInst with CatchPadInst predecessor cannot have any other "
+             "predecessors.",
+             &CPI);
+    }
+
   BasicBlock *UnwindDest = CPI.getUnwindDest();
   Instruction *I = UnwindDest->getFirstNonPHI();
   Assert(
@@ -2946,14 +2937,6 @@ void Verifier::visitCleanupPadInst(CleanupPadInst &CPI) {
 
   BasicBlock *BB = CPI.getParent();
 
-  if (!CleanupPadResultTy)
-    CleanupPadResultTy = CPI.getType();
-  else
-    Assert(CleanupPadResultTy == CPI.getType(),
-           "The cleanuppad instruction should have a consistent result type "
-           "inside a function.",
-           &CPI);
-
   Function *F = BB->getParent();
   Assert(F->hasPersonalityFn(),
          "CleanupPadInst needs to be in a function with a personality.", &CPI);
@@ -2964,6 +2947,18 @@ void Verifier::visitCleanupPadInst(CleanupPadInst &CPI) {
          "CleanupPadInst not the first non-PHI instruction in the block.",
          &CPI);
 
+  CleanupReturnInst *FirstCRI = nullptr;
+  for (User *U : CPI.users())
+    if (CleanupReturnInst *CRI = dyn_cast<CleanupReturnInst>(U)) {
+      if (!FirstCRI)
+        FirstCRI = CRI;
+      else
+        Assert(CRI->getUnwindDest() == FirstCRI->getUnwindDest(),
+               "Cleanuprets from same cleanuppad have different exceptional "
+               "successors.",
+               FirstCRI, CRI);
+    }
+
   visitInstruction(CPI);
 }