CreateCont opcode cleanup - removing FillContLocals instruction

FillContLocals was used only when num of locals > kMaxInlineContLocals; it doesn't hurt performance to copy them always within hhir.

hphp/doc/ir.specification

@@ -808,7 +808,7 @@ D:FuncCtx = LdClsMethodFCache S0:ConstStr S1:ConstStr
   names of the callee's class and method, respectively.  S2 is the
   current context.  This instruction loads the corresponding entry in
   the MethodFCache if needed.  In case the given method is not found,
-  control is transferred to label L.  The current frame pointer is 
+  control is transferred to label L.  The current frame pointer is
   passed in S3.
 
 D:Ctx = GetCtxFwdCall S0:Ctx S1:Func
@@ -1509,13 +1509,6 @@ D:Obj = CreateContMeth S0:ConstFunc S1:ConstFunc S2:Ctx
      S1 - the generator function
      S2 - the m_this/m_cls field of the current frame pointer
 
-FillContLocals S0:FramePtr S1:ConstFunc S2:ConstFunc S3:Obj
-
-  Fill the locals for the continuation object S3.  Copies the values
-  for the locals from the frame S0 to their heap location in the
-  continuation object.  S1 is the "original" func, and S2 is the
-  generator function.
-
 ContEnter S0:FramePtr S1:TCA S2:ConstInt S3:FramePtr
 
   Enters a generator body. S0 is the ActRec embedded in the Continuation

hphp/runtime/vm/jit/codegen.cpp

@@ -399,7 +399,6 @@ CALL_OPCODE(ConvCellToStr);
 
 CALL_OPCODE(CreateContFunc)
 CALL_OPCODE(CreateContMeth)
-CALL_OPCODE(FillContLocals)
 CALL_OPCODE(NewArray)
 CALL_OPCODE(NewTuple)
 CALL_OPCODE(AllocObj)

hphp/runtime/vm/jit/hhbctranslator.cpp

@@ -1008,25 +1008,21 @@ void HhbcTranslator::emitCIterFree(uint32_t iterId) {
 
 typedef std::map<int, int> ContParamMap;
 /*
- * mapContParams determines if every named local in origFunc has a
- * corresponding named local in genFunc. If this step succeeds and
+ * mapContParams builds a mapping between named locals in origFunc and
+ * corresponding named locals in genFunc. If this step succeeds and
  * there's no VarEnv at runtime, the continuation's variables can be
  * filled completely inline in the TC (assuming there aren't too
  * many).
  */
 static
-bool mapContParams(ContParamMap& map,
+void mapContParams(ContParamMap& map,
                    const Func* origFunc, const Func* genFunc) {
   const StringData* const* varNames = origFunc->localNames();
   for (Id i = 0; i < origFunc->numNamedLocals(); ++i) {
     Id id = genFunc->lookupVarId(varNames[i]);
-    if (id != kInvalidId) {
-      map[i] = id;
-    } else {
-      return false;
-    }
+    assert(id != kInvalidId);
+    map[i] = id;
   }
-  return true;
 }
 
 void HhbcTranslator::emitCreateCont(Id funNameStrId) {
@@ -1051,34 +1047,30 @@ void HhbcTranslator::emitCreateCont(Id funNameStrId) {
       );
 
   ContParamMap params;
-  if (origLocals <= Translator::kMaxInlineContLocals &&
-      mapContParams(params, origFunc, genFunc)) {
-    static auto const thisStr = StringData::GetStaticString("this");
-    Id thisId = kInvalidId;
-    const bool fillThis = origFunc->isMethod() &&
-      !origFunc->isStatic() &&
-      ((thisId = genFunc->lookupVarId(thisStr)) != kInvalidId) &&
-      (origFunc->lookupVarId(thisStr) == kInvalidId);
+  mapContParams(params, origFunc, genFunc);
 
-    SSATmp* contAR = gen(
-      LdRaw, Type::PtrToGen, cont, cns(RawMemSlot::ContARPtr));
-    for (int i = 0; i < origLocals; ++i) {
-      // We must generate an AssertLoc because we don't have tracelet
-      // guards on the object type in these outer generator functions.
-      gen(AssertLoc, Type::Gen, LocalId(i), m_tb->fp());
-      // Copy the value of the local to the cont object and set the
-      // local to uninit so that we don't need to change refcounts.
-      gen(StMem, contAR, cns(-cellsToBytes(params[i] + 1)), ldLoc(i));
-      gen(StLoc, LocalId(i), m_tb->fp(), m_tb->genDefUninit());
-    }
-    if (fillThis) {
-      assert(thisId != kInvalidId);
-      auto const thisObj = gen(IncRef, gen(LdThis, m_tb->fp()));
-      gen(StMem, contAR, cns(-cellsToBytes(thisId + 1)), thisObj);
-    }
-  } else {
-    gen(FillContLocals, m_tb->fp(), cns(origFunc),
-      cns(genFunc), cont);
+  static auto const thisStr = StringData::GetStaticString("this");
+  Id thisId = kInvalidId;
+  const bool fillThis = origFunc->isMethod() &&
+    !origFunc->isStatic() &&
+    ((thisId = genFunc->lookupVarId(thisStr)) != kInvalidId) &&
+    (origFunc->lookupVarId(thisStr) == kInvalidId);
+
+  SSATmp* contAR = gen(
+    LdRaw, Type::PtrToGen, cont, cns(RawMemSlot::ContARPtr));
+  for (int i = 0; i < origLocals; ++i) {
+    // We must generate an AssertLoc because we don't have tracelet
+    // guards on the object type in these outer generator functions.
+    gen(AssertLoc, Type::Gen, LocalId(i), m_tb->fp());
+    // Copy the value of the local to the cont object and set the
+    // local to uninit so that we don't need to change refcounts.
+    gen(StMem, contAR, cns(-cellsToBytes(params[i] + 1)), ldLoc(i));
+    gen(StLoc, LocalId(i), m_tb->fp(), m_tb->genDefUninit());
+  }
+  if (fillThis) {
+    assert(thisId != kInvalidId);
+    auto const thisObj = gen(IncRef, gen(LdThis, m_tb->fp()));
+    gen(StMem, contAR, cns(-cellsToBytes(thisId + 1)), thisObj);
   }
 
   push(cont);

hphp/runtime/vm/jit/ir.h

@@ -472,10 +472,6 @@ O(Spill,                       DofS(0), SUnk,                            Mem) \
 O(Reload,                      DofS(0), SUnk,                            Mem) \
 O(CreateContFunc,               D(Obj), C(Func) C(Func),             E|N|PRc) \
 O(CreateContMeth,               D(Obj), C(Func) C(Func) S(Ctx),      E|N|PRc) \
-O(FillContLocals,                   ND, S(FramePtr)                           \
-                                          C(Func)                             \
-                                          C(Func)                             \
-                                          S(Obj),                    E|N|Mem) \
 O(ContEnter,                        ND, S(FramePtr)                           \
                                           S(TCA) C(Int) S(FramePtr),   E|Mem) \
 O(ContPreNext,                      ND, S(Obj),                        E|Mem) \

hphp/runtime/vm/jit/nativecalls.cpp

@@ -177,9 +177,6 @@ static CallMap s_callMap({
                          {{SSA, 0}, {SSA, 1}}},
     {CreateContMeth,     (TCA)&VMExecutionContext::createContMeth, DSSA, SNone,
                          {{SSA, 0}, {SSA, 1}, {SSA, 2}}},
-    {FillContLocals, (TCA)&VMExecutionContext::fillContinuationVars,
-              DNone, SNone,
-              {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}}},
 
     /* VectorTranslator helpers */
     {BaseG,    {FSSA, 0}, DSSA, SSync, {{TV, 1}, {SSA, 2}}},

hphp/runtime/vm/jit/translator.h

@@ -680,7 +680,6 @@ public:
   // kMaxInlineReturnDecRefs is the maximum ref-counted locals to
   // generate an inline return for.
   static const int kMaxInlineReturnDecRefs = 1;
-  static const int kMaxInlineContLocals = 10;
 
 private:
   friend struct TraceletContext;