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

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: docs/LangRef.rst:5197-5198
@@ +5196,4 @@
+-  It is undefined behavior for control to transfer from a ``catchpad`` to
+   itself without first executing a ``catchret`` that consumes the
+   ``catchpad``'s value (counting instances in of the same ``catchpad`` in
+   different call frames as distinct).
----------------
s/consumes the catchpad's value/consumes the catchpad/ maybe?

I don't think the parenthetical makes this any clearer. WDYT?

================
Comment at: include/llvm/IR/Instructions.h:3581-3584
@@ -3580,6 +3580,6 @@
   void init(Value *RetVal, BasicBlock *UnwindBB);
-  CleanupReturnInst(LLVMContext &C, Value *RetVal, BasicBlock *UnwindBB,
-                    unsigned Values, Instruction *InsertBefore = nullptr);
-  CleanupReturnInst(LLVMContext &C, Value *RetVal, BasicBlock *UnwindBB,
-                    unsigned Values, BasicBlock *InsertAtEnd);
+  CleanupReturnInst(Value *CleanupPad, BasicBlock *UnwindBB, unsigned Values,
+                    Instruction *InsertBefore = nullptr);
+  CleanupReturnInst(Value *CleanupPad, BasicBlock *UnwindBB, unsigned Values,
+                    BasicBlock *InsertAtEnd);
 
----------------
Should we tighten up these Value* types to be CleanupBlockInst*?

================
Comment at: include/llvm/IR/Instructions.h:3622-3623
@@ -3645,1 +3621,4 @@
+  /// Convenience accessor.
+  Value *getCleanupPad() const { return Op<-1>(); }
+  void setCleanupPad(Value *CleanupPad) { Op<-1>() = CleanupPad; }
 
----------------
Regardless of whether we tighten the constructor, these should probably use CleanupPadInst* instead of Value*. Use cast<> to pull it out, it'll assert if things go wrong.

================
Comment at: include/llvm/IR/Instructions.h:4022-4025
@@ -4039,7 +4021,6 @@
 private:
-  void init(BasicBlock *BB, Value *RetVal);
-  CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values,
+  void init(Value *CatchPad, BasicBlock *BB);
+  CatchReturnInst(Value *CatchPad, BasicBlock *BB,
                   Instruction *InsertBefore = nullptr);
-  CatchReturnInst(BasicBlock *BB, Value *RetVal, unsigned Values,
-                  BasicBlock *InsertAtEnd);
+  CatchReturnInst(Value *CatchPad, BasicBlock *BB, BasicBlock *InsertAtEnd);
 
----------------
Ditto, should we be more strict, or does that have wide-ranging complexity? At the very least, we should assert in init if we don't already.

================
Comment at: include/llvm/IR/Instructions.h:4048-4049
@@ -4072,5 +4047,4 @@
   /// Convenience accessors.
-  BasicBlock *getSuccessor() const { return cast<BasicBlock>(Op<-1>()); }
-  void setSuccessor(BasicBlock *NewSucc) { Op<-1>() = (Value *)NewSucc; }
-  unsigned getNumSuccessors() const { return 1; }
+  Value *getCatchPad() const { return Op<0>(); }
+  void setCatchPad(Value *CatchPad) { Op<0>() = CatchPad; }
 
----------------
Ditto, we should tighten the accesssors.


http://reviews.llvm.org/D12108