From eddef7ffc0404f43e8edf276e4c51db0713b21e3 Mon Sep 17 00:00:00 2001
From: Mirek Klimos <miro@fb.com>
Date: Thu, 27 Jun 2013 17:42:28 -0700
Subject: [PATCH] Replacing PackCont and ContExit opcodes with ContSuspend.

PackCont opcode is always followed by ContExit so let's join them into ContSuspend. ContResume counterpart may come later (replacing ContNext/Send/Raise/Enter).
---
 hphp/compiler/analysis/emitter.cpp            |  9 +++-----
 hphp/doc/bytecode.specification               | 21 +++++++------------
 hphp/doc/ir.specification                     |  2 +-
 .../runtime/debugger/cmd/cmd_flow_control.cpp |  2 +-
 hphp/runtime/debugger/cmd/cmd_next.cpp        | 16 +++++++-------
 hphp/runtime/vm/bytecode.cpp                  | 18 +++++++---------
 hphp/runtime/vm/hhbc.h                        |  3 +--
 hphp/runtime/vm/jit/hhbctranslator.cpp        | 18 ++++++++--------
 hphp/runtime/vm/jit/hhbctranslator.h          |  5 ++---
 hphp/runtime/vm/jit/irtranslator.cpp          |  8 ++-----
 hphp/runtime/vm/jit/translator-instrs.h       |  3 +--
 hphp/runtime/vm/jit/translator.cpp            |  7 +++----
 hphp/runtime/vm/jit/translator.h              |  2 +-
 13 files changed, 47 insertions(+), 67 deletions(-)

diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp
index 7a7cc2d74..60667a706 100644
--- a/hphp/compiler/analysis/emitter.cpp
+++ b/hphp/compiler/analysis/emitter.cpp
@@ -3941,17 +3941,14 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) {
         visit(y->getExpression());
         emitConvertToCell(e);
 
-        // pack continuation and set the return label
+        // suspend continuation and set the return label
         assert(m_evalStack.size() == 1);
         m_metaInfo.addKnownDataType(
           KindOfObject, false, m_ue.bcPos(), false, 1);
-        e.PackCont(2 * y->getLabel());
-
-        // transfer control
-        assert(m_evalStack.size() == 0);
-        e.ContExit();
+        e.ContSuspend(2 * y->getLabel());
 
         // emit return label for raise()
+        assert(m_evalStack.size() == 0);
         e.Null();
         m_yieldLabels[2 * y->getLabel() - 1].set(e);
 
diff --git a/hphp/doc/bytecode.specification b/hphp/doc/bytecode.specification
index 24c25671f..64d5a9603 100644
--- a/hphp/doc/bytecode.specification
+++ b/hphp/doc/bytecode.specification
@@ -3683,8 +3683,8 @@ ArrayIdx    [C C C] -> [C]
 CreateCont <function name>    []  ->  [C]
 
   Creates a Continuation object and pushes it on the stack. The Continuation
-  will capture all defined local variables in the current function. The 
-  Continuation will store a reference to the function named by the string 
+  will capture all defined local variables in the current function. The
+  Continuation will store a reference to the function named by the string
   immediate to be used as its body.
 
 ContEnter [C]  ->  []
@@ -3694,11 +3694,13 @@ ContEnter [C]  ->  []
   associated with the $this object of the Continuation object. The value on the
   stack is sent to the Continuation to be retrieved by UnpackCont.
 
-ContExit    []  ->  []
+ContSuspend  <label id>   [C]  ->  []
 
-  This instruction may only appear in continuation bodies. It transfers control
-  flow to the caller of the continuation body, which must be a non-static method
-  of the Continuation class.
+  Suspend continuation. This instruction may only appear in continuation bodies.
+  Packs all defined local variables into the Continuation object in local 0.
+  Store $1 in the continuation as the result of the current iteration and
+  set the continuation's label to <label id>. Transfer control flow to the
+  caller of the continuation body, which must be a non-static method of the Continuation class.
 
 UnpackCont    []  ->  [C:Int C]
 
@@ -3706,13 +3708,6 @@ UnpackCont    []  ->  [C:Int C]
   0 into the containing environment. The stack will contain the current label of
   the continuation in $1 and the value sent to the continuation in $2. The value
   will be no longer referenced by the Continuation object.
-  
-PackCont <label id>    [C]  ->  []
-
-  Pack continuation. Packs all defined local variables into the Continuation
-  object in local 0. The value on the top of the stack is stored in the
-  continuation as the result of the current iteration and the continuation's
-  label is set to <label id>.
 
 ContRetC    [C]  ->  []
 
diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification
index 6635e5792..0adcee1e0 100644
--- a/hphp/doc/ir.specification
+++ b/hphp/doc/ir.specification
@@ -405,7 +405,7 @@ D:StkPtr = CastStk<T,offset> S0:StkPtr
 D:StkPtr = CoerceStk<T,offset> S0:StkPtr -> L
 
   Returns a new StkPtr that represents the same stack as S0, but with
-  the slot at offset (in cells) converted to type T. If the type conversion 
+  the slot at offset (in cells) converted to type T. If the type conversion
   can't be done then branches to label L.
 
 CheckInit S0:Gen -> L
diff --git a/hphp/runtime/debugger/cmd/cmd_flow_control.cpp b/hphp/runtime/debugger/cmd/cmd_flow_control.cpp
index 3643afe3e..f4923bc6c 100644
--- a/hphp/runtime/debugger/cmd/cmd_flow_control.cpp
+++ b/hphp/runtime/debugger/cmd/cmd_flow_control.cpp
@@ -107,7 +107,7 @@ void CmdFlowControl::installLocationFilterForLine(InterruptSite *site) {
     }
   }
   auto excludeContinuationReturns = [] (Op op) {
-    return (op != OpContExit) && (op != OpContRetC);
+    return (op != OpContSuspend) && (op != OpContRetC);
   };
   g_vmContext->m_lastLocFilter->addRanges(unit, ranges,
                                           excludeContinuationReturns);
diff --git a/hphp/runtime/debugger/cmd/cmd_next.cpp b/hphp/runtime/debugger/cmd/cmd_next.cpp
index 1ebfc1412..adaa43f20 100644
--- a/hphp/runtime/debugger/cmd/cmd_next.cpp
+++ b/hphp/runtime/debugger/cmd/cmd_next.cpp
@@ -178,12 +178,12 @@ void CmdNext::stepCurrentLine(CmdInterrupt& interrupt, ActRec* fp, PC pc) {
   // land back at the callsite of send(). For returns from generators, we follow
   // the execution stack for now, and end up at the caller of ASIO or send().
   if (fp->m_func->isGenerator() &&
-      ((*pc == OpContExit) || (*pc == OpContRetC))) {
+      ((*pc == OpContSuspend) || (*pc == OpContRetC))) {
     TRACE(2, "CmdNext: encountered yield or return from generator\n");
     // Patch the projected return point(s) in both cases, to catch if we exit
     // the the asio iterator or if we are being iterated directly by PHP.
     setupStepOuts();
-    if (*pc == OpContExit) {
+    if (*pc == OpContSuspend) {
       // Patch the next normal execution point so we can pickup the stepping
       // from there if the caller is C++.
       setupStepCont(fp, pc);
@@ -204,18 +204,18 @@ bool CmdNext::atStepContOffset(Unit* unit, Offset o) {
   return (unit == m_stepContUnit) && (o == m_stepContOffset);
 }
 
-// A ContExit is followed by code to support ContRaise, then code for
+// A ContSuspend is followed by code to support ContRaise, then code for
 // ContSend/ContNext. We want to continue stepping on the latter. The normal
 // exception handling logic will take care of the former.
 // This logic is sensitive to the code gen here... we don't have access to the
 // offsets for the labels used to generate this code, so we rely on the
 // simplicity of the exceptional path.
 void CmdNext::setupStepCont(ActRec* fp, PC pc) {
-  assert(*pc == OpContExit); // One byte
-  assert(*(pc+1) == OpNull); // One byte
-  assert(*(pc+2) == OpThrow); // One byte
-  assert(*(pc+3) == OpNull); // One byte
-  Offset nextInst = fp->m_func->unit()->offsetOf(pc + 4);
+  assert(*pc == OpContSuspend); // One byte + one byte argument
+  assert(*(pc+2) == OpNull); // One byte
+  assert(*(pc+3) == OpThrow); // One byte
+  assert(*(pc+4) == OpNull); // One byte
+  Offset nextInst = fp->m_func->unit()->offsetOf(pc + 5);
   m_stepContUnit = fp->m_func->unit();
   m_stepContOffset = nextInst;
   m_stepContTag = getContinuationTag(fp);
diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp
index bf5a58315..40d768b50 100644
--- a/hphp/runtime/vm/bytecode.cpp
+++ b/hphp/runtime/vm/bytecode.cpp
@@ -6698,7 +6698,7 @@ static inline void setContVar(const Func* genFunc,
     if (!contFP->hasVarEnv()) {
       // We pass skipInsert to this VarEnv because it's going to exist
       // independent of the chain; i.e. we can't stack-allocate it. We link it
-      // into the chain in UnpackCont, and take it out in PackCont.
+      // into the chain in UnpackCont, and take it out in ContSuspend.
       contFP->setVarEnv(VarEnv::createLocalOnHeap(contFP));
     }
     contFP->getVarEnv()->setWithRef(name, src);
@@ -6807,15 +6807,6 @@ void VMExecutionContext::iopContEnter(PC& pc) {
   }
 }
 
-void VMExecutionContext::iopContExit(PC& pc) {
-  NEXT();
-
-  EventHook::FunctionExit(m_fp);
-  ActRec* prevFp = m_fp->arGetSfp();
-  pc = prevFp->m_func->getEntry() + m_fp->m_soff;
-  m_fp = prevFp;
-}
-
 inline void OPTBLD_INLINE VMExecutionContext::iopUnpackCont(PC& pc) {
   NEXT();
   c_Continuation* cont = frame_continuation(m_fp);
@@ -6829,13 +6820,18 @@ inline void OPTBLD_INLINE VMExecutionContext::iopUnpackCont(PC& pc) {
   label->m_data.num = cont->m_label;
 }
 
-inline void OPTBLD_INLINE VMExecutionContext::iopPackCont(PC& pc) {
+inline void OPTBLD_INLINE VMExecutionContext::iopContSuspend(PC& pc) {
   NEXT();
   DECODE_IVA(label);
   c_Continuation* cont = frame_continuation(m_fp);
 
   cont->c_Continuation::t_update(label, tvAsCVarRef(m_stack.topTV()));
   m_stack.popTV();
+
+  EventHook::FunctionExit(m_fp);
+  ActRec* prevFp = m_fp->arGetSfp();
+  pc = prevFp->m_func->getEntry() + m_fp->m_soff;
+  m_fp = prevFp;
 }
 
 inline void OPTBLD_INLINE VMExecutionContext::iopContRetC(PC& pc) {
diff --git a/hphp/runtime/vm/hhbc.h b/hphp/runtime/vm/hhbc.h
index 120f74e21..4890d7888 100644
--- a/hphp/runtime/vm/hhbc.h
+++ b/hphp/runtime/vm/hhbc.h
@@ -548,9 +548,8 @@ enum SetOpOp {
   O(CreateCl,        TWO(IVA,SA),      CVMANY,          ONE(CV),    NF) \
   O(CreateCont,      ONE(SA),          NOV,             ONE(CV),    NF) \
   O(ContEnter,       NA,               ONE(CV),         NOV,        CF) \
-  O(ContExit,        NA,               NOV,             NOV,        CF) \
   O(UnpackCont,      NA,               NOV,             TWO(CV,CV), NF) \
-  O(PackCont,        ONE(IVA),         ONE(CV),         NOV,        NF) \
+  O(ContSuspend,     ONE(IVA),         ONE(CV),         NOV,        CF) \
   O(ContRetC,        NA,               ONE(CV),         NOV,        CF_TF) \
   O(ContCheck,       ONE(IVA),         NOV,             NOV,        NF) \
   O(ContSend,        NA,               NOV,             ONE(CV),    NF) \
diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp
index 6a7d932c1..5d1b88855 100644
--- a/hphp/runtime/vm/jit/hhbctranslator.cpp
+++ b/hphp/runtime/vm/jit/hhbctranslator.cpp
@@ -1101,7 +1101,7 @@ void HhbcTranslator::emitContEnter(int32_t returnBcOffset) {
   assert(m_stackDeficit == 0);
 }
 
-void HhbcTranslator::emitContExitImpl() {
+void HhbcTranslator::emitContReturnControl() {
   auto const retAddr = gen(LdRetAddr, m_tb->fp());
   auto const fp = gen(FreeActRec, m_tb->fp());
   auto const sp = spillStack();
@@ -1109,11 +1109,6 @@ void HhbcTranslator::emitContExitImpl() {
   m_hasExit = true;
 }
 
-void HhbcTranslator::emitContExit() {
-  gen(ExitWhenSurprised, getExitSlowTrace());
-  emitContExitImpl();
-}
-
 void HhbcTranslator::emitUnpackCont() {
   gen(AssertLoc, Type::Obj, LocalId(0), m_tb->fp());
   auto const cont = ldLoc(0);
@@ -1121,7 +1116,9 @@ void HhbcTranslator::emitUnpackCont() {
   push(gen(LdRaw, Type::Int, cont, cns(RawMemSlot::ContLabel)));
 }
 
-void HhbcTranslator::emitPackCont(int64_t labelId) {
+void HhbcTranslator::emitContSuspend(int64_t labelId) {
+  gen(ExitWhenSurprised, getExitSlowTrace());
+
   gen(AssertLoc, Type::Obj, LocalId(0), m_tb->fp());
   auto const cont = ldLoc(0);
   auto const newVal = popC();
@@ -1131,6 +1128,9 @@ void HhbcTranslator::emitPackCont(int64_t labelId) {
   gen(
     StRaw, cont, cns(RawMemSlot::ContLabel), cns(labelId)
   );
+
+  // transfer control
+  emitContReturnControl();
 }
 
 void HhbcTranslator::emitContRetC() {
@@ -1146,7 +1146,7 @@ void HhbcTranslator::emitContRetC() {
   gen(DecRef, oldVal);
 
   // transfer control
-  emitContExitImpl();
+  emitContReturnControl();
 }
 
 void HhbcTranslator::emitContCheck(bool checkStarted) {
@@ -3311,7 +3311,7 @@ Type setOpResult(Type locType, Type valType, SetOpOp op) {
 uint32_t localOutputId(const NormalizedInstruction& inst) {
   switch (inst.op()) {
     case OpUnpackCont:
-    case OpPackCont:
+    case OpContSuspend:
     case OpContRetC:
     case OpContSend:
     case OpContRaise:
diff --git a/hphp/runtime/vm/jit/hhbctranslator.h b/hphp/runtime/vm/jit/hhbctranslator.h
index 4661c067d..5b68c622c 100644
--- a/hphp/runtime/vm/jit/hhbctranslator.h
+++ b/hphp/runtime/vm/jit/hhbctranslator.h
@@ -375,10 +375,9 @@ struct HhbcTranslator {
   // continuations
   void emitCreateCont(Id funNameStrId);
   void emitContEnter(int32_t returnBcOffset);
-  void emitContExitImpl();
-  void emitContExit();
   void emitUnpackCont();
-  void emitPackCont(int64_t labelId);
+  void emitContReturnControl();
+  void emitContSuspend(int64_t labelId);
   void emitContRetC();
   void emitContCheck(bool checkStarted);
   void emitContSend();
diff --git a/hphp/runtime/vm/jit/irtranslator.cpp b/hphp/runtime/vm/jit/irtranslator.cpp
index 40db55e83..3cded2448 100644
--- a/hphp/runtime/vm/jit/irtranslator.cpp
+++ b/hphp/runtime/vm/jit/irtranslator.cpp
@@ -548,16 +548,12 @@ void Translator::translateContEnter(const NormalizedInstruction& i) {
   HHIR_EMIT(ContEnter, callOffsetInUnit);
 }
 
-void Translator::translateContExit(const NormalizedInstruction& i) {
-  HHIR_EMIT(ContExit);
-}
-
 void Translator::translateUnpackCont(const NormalizedInstruction& i) {
   HHIR_EMIT(UnpackCont);
 }
 
-void Translator::translatePackCont(const NormalizedInstruction& i) {
-  HHIR_EMIT(PackCont, i.imm[0].u_IVA);
+void Translator::translateContSuspend(const NormalizedInstruction& i) {
+  HHIR_EMIT(ContSuspend, i.imm[0].u_IVA);
 }
 
 void Translator::translateContRetC(const NormalizedInstruction& i) {
diff --git a/hphp/runtime/vm/jit/translator-instrs.h b/hphp/runtime/vm/jit/translator-instrs.h
index 0dd568a0a..5d574f18a 100644
--- a/hphp/runtime/vm/jit/translator-instrs.h
+++ b/hphp/runtime/vm/jit/translator-instrs.h
@@ -129,9 +129,8 @@
   CASE(CreateCl) \
   CASE(CreateCont) \
   CASE(ContEnter) \
-  CASE(ContExit) \
   CASE(UnpackCont) \
-  CASE(PackCont) \
+  CASE(ContSuspend) \
   CASE(ContRetC) \
   CASE(ContCheck) \
   CASE(ContSend) \
diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp
index 4263d3d0e..06fa14121 100644
--- a/hphp/runtime/vm/jit/translator.cpp
+++ b/hphp/runtime/vm/jit/translator.cpp
@@ -1265,9 +1265,8 @@ static const struct {
 
   { OpCreateCont,  {None,             Stack1,       OutObject,         1 }},
   { OpContEnter,   {Stack1,           None,         OutNone,          -1 }},
-  { OpContExit,    {None,             None,         OutNone,           0 }},
   { OpUnpackCont,  {Local,            StackTop2,    OutInt64,          2 }},
-  { OpPackCont,    {Local|Stack1,     None,         OutNone,          -1 }},
+  { OpContSuspend, {Local|Stack1,     None,         OutNone,          -1 }},
   { OpContRetC,    {Local|Stack1,     None,         OutNone,          -1 }},
   { OpContCheck,   {None,             None,         OutNone,           0 }},
   { OpContSend,    {Local,            Stack1,       OutUnknown,        1 }},
@@ -1837,7 +1836,7 @@ void Translator::getInputs(SrcKey startSk,
     auto insertAt = inputs.end();
     switch (ni->op()) {
       case OpUnpackCont:
-      case OpPackCont:
+      case OpContSuspend:
       case OpContRetC:
       case OpContSend:
       case OpContRaise:
@@ -2672,7 +2671,7 @@ Translator::getOperandConstraintCategory(NormalizedInstruction* instr,
     case OpPopR:
       return DataTypeCountness;
 
-    case OpPackCont:
+    case OpContSuspend:
     case OpContRetC:
       // The stack input is teleported to the continuation's m_value field
       return opndIdx == 0 ? DataTypeGeneric : DataTypeSpecific;
diff --git a/hphp/runtime/vm/jit/translator.h b/hphp/runtime/vm/jit/translator.h
index 5450535e1..654030602 100644
--- a/hphp/runtime/vm/jit/translator.h
+++ b/hphp/runtime/vm/jit/translator.h
@@ -985,7 +985,7 @@ opcodeControlFlowInfo(const Op instr) {
     case OpJmpNZ:
     case OpSwitch:
     case OpSSwitch:
-    case OpContExit:
+    case OpContSuspend:
     case OpContRetC:
     case OpRetC:
     case OpRetV: