[llvm-commits] [Patch] Resume Instruction

[Chris Lattner]

On Jul 29, 2011, at 11:41 PM, Bill Wendling wrote:

> This patch defines the 'resume' instruction and includes parsing of IR and bitcode.
> 
> Please review.

+class ResumeInst : public TerminatorInst {
+  ResumeInst(const ResumeInst &RI);
+
+  explicit ResumeInst(LLVMContext &C, Value *Exn, Instruction *InsertBefore=0);
+  ResumeInst(LLVMContext &C, Value *Exn, BasicBlock *InsertAtEnd);

Exn is required, right?  (I missed it or this wasn't clear in your langref patch, please clarify it there if needed).  Please remove the Context arguments from these and the ::Create methods.

+  Value *getResumeValue() const { return Op<0>(); }

This should just be "getValue()".

+++ lib/Target/CBackend/CBackend.cpp	(working copy)
@@ -288,10 +288,12 @@
     void visitInvokeInst(InvokeInst &I) {
       llvm_unreachable("Lowerinvoke pass didn't work!");
     }
-
     void visitUnwindInst(UnwindInst &I) {
       llvm_unreachable("Lowerinvoke pass didn't work!");
     }
+    void visitResumeInst(ResumeInst &I) {
+      llvm_unreachable("Lowerinvoke pass didn't work!");
+    }

This doesn't seem sufficient: LowerInvoke isn't handling resume in your patch.  I'll assume that is coming later.

+++ lib/VMCore/Instructions.cpp	(working copy)

+ResumeInst::ResumeInst(const ResumeInst &RI)
+  : TerminatorInst(Type::getVoidTy(RI.getContext()), Instruction::Resume,
+                   OperandTraits<ResumeInst>::op_begin(this), 1) {
+  Op<0>() = RI.Op<0>();
+  SubclassOptionalData = RI.SubclassOptionalData;
+}

Resumes have no SubclassOptionalData to copy.  Please remove that line.


+++ lib/VMCore/AsmWriter.cpp	(working copy)
@@ -1731,6 +1731,9 @@
       writeOperand(I.getOperand(i), true);
     }
     Out << ']';
+  } else if (isa<ResumeInst>(I)) {
+    Out << ' ';
+    writeOperand(Operand, true);
   } else if (const PHINode *PN = dyn_cast<PHINode>(&I)) {
     Out << ' ';
     TypePrinter.print(I.getType(), Out);


This patch doesn't look needed.

Otherwise, this looks good.  Please feel free to commit this (with the Context argument's removed) after we settle on the LangRef patch.

-Chris