Clean-up GenericRetDecRefs

This diff cleans some register shuffling that GenericRetDecRefs was
doing because it calls helpers that use a special ABI, in which r13 (a
callee-saved register) is not saved.

The new approach simply checks if r13 is live, and if so, pushes and
pops it around the call. This removes the need for GenericRetDecRefs
to take the function return value as a source.

hphp/doc/ir.specification

@@ -954,21 +954,17 @@ RaiseWarning S0:Str
 
   Raises a warning with the text in S0 as its message.
 
-D:StkPtr = GenericRetDecRefs S0:FramePtr S1:Gen S2:ConstInt
+D:StkPtr = GenericRetDecRefs S0:FramePtr S1:ConstInt
 
   Does decrefs of all the current function's locals, where S0 is a
-  pointer to the relevant activation record, and S2 is the number of
+  pointer to the relevant activation record, and S1 is the number of
   locals in the current function.
 
-  S1 is the return value for the function, which GenericRetDecRefs
-  needs access to currently only to ensure it isn't clobbered.
-
   Returns the adjusted VM stack pointer (pointing at the return value
   location).
 
   Semantically similar to a series of DecRefLoc followed by
-  RetAdjustStack.  Note that this does not store the return value even
-  though it takes it as a source.
+  RetAdjustStack.
 
 
 10. Stores

hphp/runtime/vm/translator/hopt/codegen.cpp

@@ -2714,88 +2714,39 @@ void CodeGenerator::cgDecRefLoc(IRInstruction* inst) {
 
 void CodeGenerator::cgGenericRetDecRefs(IRInstruction* inst) {
   auto const rFp       = m_regs[inst->getSrc(0)].getReg();
-  auto const retVal    = inst->getSrc(1);
-  auto const numLocals = inst->getSrc(2)->getValInt();
+  auto const numLocals = inst->getSrc(1)->getValInt();
   auto const rDest     = m_regs[inst->getDst()].getReg();
   auto& a = m_as;
 
-  RegSet retvalRegs = m_regs[retVal].getRegs();
-  assert(retvalRegs.size() <= 2);
-
-  /*
-   * The generic decref helpers preserve these two registers.
-   *
-   * XXX/TODO: we ideally shouldn't be moving the return value into
-   * these regs and then back; it'd be better to precolor allocation
-   * this way, or failing that remap the return value SSATmp with a
-   * separate instruction.  This scheme also won't work for us once we
-   * want hackIR to support handling surprise flags, because
-   * setprofile will look on the VM stack for the return value.
-   */
-  auto spillRegs = RegSet(r14).add(r15);
-
-  /*
-   * Since we're making a call using a custom ABI to the generic
-   * decref helper, it's important that our src and dest registers are
-   * allocated to the registers we expect, and that no other SSATmp's
-   * are still allocated to registers at this time.
-   */
-  const auto UNUSED expectedLiveRegs = RegSet(rFp).add(rDest) | retvalRegs;
-  assert((m_state.liveRegs[m_curInst] - expectedLiveRegs).empty());
   assert(rFp == rVmFp &&
          "free locals helper assumes the frame pointer is rVmFp");
   assert(rDest == rVmSp &&
          "free locals helper adjusts rVmSp, which must be our dst reg");
 
-  if (!numLocals) {
-    a.  lea   (rFp[AROFF(m_r)], rDest);
-    return;
-  }
+  if (numLocals == 0) return;
 
-  // Remove overlap so we don't move registers that are already in the
-  // saved set.
-  auto intersectedRegs = spillRegs & retvalRegs;
-  spillRegs -= intersectedRegs;
-  retvalRegs -= intersectedRegs;
+  // The helpers called below use a special ABI, in which r13 is not saved.
+  // So save r13 on the stack if it's live.
+  bool saveR13 = m_state.liveRegs[inst].contains(r13);
 
-  auto grabPair = [&] (std::pair<PhysReg,PhysReg>& out) {
-    assert(!retvalRegs.empty() && !spillRegs.empty());
-    retvalRegs.findFirst(out.first);
-    spillRegs.findFirst(out.second);
-    retvalRegs.remove(out.first);
-    spillRegs.remove(out.second);
-  };
-
-  auto savePairA = std::make_pair(InvalidReg, InvalidReg);
-  auto savePairB = std::make_pair(InvalidReg, InvalidReg);
-  if (!retvalRegs.empty()) {
-    grabPair(savePairA);
-    if (!retvalRegs.empty()) {
-      grabPair(savePairB);
-    }
-  }
-
-  if (savePairA.first != InvalidReg) {
-    a.  movq   (savePairA.first, savePairA.second);
-    if (savePairB.first != InvalidReg) {
-      a.movq   (savePairB.first, savePairB.second);
-    }
+  int stackAdjust = 8;
+  if (saveR13)  {
+    a.push(r13);
+    stackAdjust = 16;
   }
 
   auto const target = numLocals > kNumFreeLocalsHelpers
     ? m_tx64->m_freeManyLocalsHelper
     : m_tx64->m_freeLocalsHelpers[numLocals - 1];
 
-  a.    subq   (0x8, rsp);  // For parity; callee does retq $0x8.
-  a.    lea    (rFp[-numLocals * sizeof(TypedValue)], rVmSp);
-  a.    call   (target);
+  a.subq(stackAdjust, rsp);  // For parity; callee does retq $0x8.
+  a.lea(rFp[-numLocals * sizeof(TypedValue)], rDest);
+  a.call(target);
   recordSyncPoint(a);
 
-  if (savePairA.first != InvalidReg) {
-    a.  movq   (savePairA.second, savePairA.first);
-    if (savePairB.first != InvalidReg) {
-      a.movq   (savePairB.second, savePairB.first);
-    }
+  if (saveR13) {
+    a.addq(8, rsp);
+    a.pop(r13);
   }
 }
 

hphp/runtime/vm/translator/hopt/hhbctranslator.cpp

@@ -2041,9 +2041,7 @@ void HhbcTranslator::emitRet(Type type, bool freeInline) {
     if (mayHaveThis(curFunc)) {
       gen(DecRefThis, m_tb->getFp());
     }
-    sp = gen(
-      GenericRetDecRefs, m_tb->getFp(), retVal, cns(curFunc->numLocals())
-    );
+    sp = gen(GenericRetDecRefs, m_tb->getFp(), cns(curFunc->numLocals()));
     gen(RetVal, m_tb->getFp(), retVal);
   }
 

hphp/runtime/vm/translator/hopt/ir.h

@@ -368,8 +368,7 @@ O(IncRef,                      DofS(0), S(Gen),                    Mem|PRc|P) \
 O(DecRefLoc,                        ND, S(FramePtr),            N|E|Mem|Refs) \
 O(DecRefStack,                      ND, S(StkPtr),              N|E|Mem|Refs) \
 O(DecRefThis,                       ND, S(FramePtr),            N|E|Mem|Refs) \
-O(GenericRetDecRefs,         D(StkPtr), S(FramePtr)                           \
-                                          S(Gen) C(Int),        E|N|Mem|Refs) \
+O(GenericRetDecRefs,         D(StkPtr), S(FramePtr) C(Int),     E|N|Mem|Refs) \
 O(DecRef,                           ND, S(Gen),           N|E|Mem|CRc|Refs|K) \
 O(DecRefMem,                        ND, S(PtrToGen)                           \
                                           C(Int),           N|E|Mem|CRc|Refs) \