Have CallFuncEntry operate on locals

Summary:
- Generate an `EnterInlineFrame` instruction for `CallFuncEntry`.
 - Introduce a variant of `EnterInlineFrame` which "teleports" stack parameters to locals. Right now this is a noop but if we want to change the layout of frame locals it will be responsible for performing this transformation.
 - Change `CallFuncEntry` to take a callee `FramePtr` defined by `DefCalleeFP` rather than a `StkPtr`.
 - Matching tweaks for prologues.

Reviewed By: jano

Differential Revision: D72466109

fbshipit-source-id: 04f8da554e05ae646de2d7e2db0a7ed24e6d2b65

Author: paulbiss

hphp/doc/ir.specification

@@ -1619,10 +1619,11 @@ To string conversions:
   Defines a new frame pointer for an ActRec at callBCOff for use in an inlined
   region. The frame is defined relative to S0, and S1 is the parent frame.
 
-| EnterInlineFrame, ND, S(FramePtr), NF
+| EnterInlineFrame<numInputs>, ND, S(FramePtr), NF
 
   Marks the start of an inlined function whose stack resides offset cells below
-  the SP.
+  the SP. If non-zero numInputs items will be teleported from the stack into the
+  locals.
 
 | LeaveInlineFrame, ND, S(FramePtr), NF
 
@@ -1687,11 +1688,11 @@ ReqBindJmp and CallFuncEntry.
 
 | CallFuncEntry<target,spOffset,arFlags>,
 |   DCall,
-|   S(StkPtr) S(FramePtr) S(Cls|Obj|Nullptr),
+|   S(FramePtr) S(FramePtr) S(Cls|Obj|Nullptr),
 |   CRc|PRc
 
   Transfer control to the func entry at `target' SrcKey, based on the pre-live
-  activation record and set of args on the stack pointed to by S0 at `spOffset'.
+  activation record and set of args in the callee frame pointed to by S0.
   S1 is the current caller frame pointer. S2 is the context (nullptr, $this or
   static::class).
 
@@ -2834,8 +2835,8 @@ ReqBindJmp and CallFuncEntry.
 
 | KillIter<iterId>, ND, S(FramePtr), NF
 
-  Mark a given ActRec, local, or iterator as being dead for the purposes of
-  memory effects. It no longer contains a meaningful value.
+  Mark a given ActRec, local, or iterator as being dead for the
+  purposes of memory effects. It no longer contains a meaningful value.
 
   These operations are used for both correctness and performance. Inserting a
   kill can help us do store-elimination; if a write is followed by a kill on

hphp/runtime/vm/jit/extra-data.h

@@ -1321,29 +1321,26 @@ struct CallData : IRExtraData {
 
 struct CallFuncEntryData : IRExtraData {
   explicit CallFuncEntryData(const SrcKey& target,
-                             IRSPRelOffset spOffset,
                              uint32_t numInitArgs,
                              uint32_t arFlags,
                              bool formingRegion)
     : target(target)
-    , spOffset(spOffset)
     , numInitArgs(numInitArgs)
     , arFlags(arFlags)
     , formingRegion(formingRegion)
   {}
 
   std::string show() const {
     return folly::sformat(
-      "{}, IRSP {}, numInitArgs {}, arFlags {}{}",
-      showShort(target), spOffset.offset, numInitArgs, arFlags,
+      "{}, numInitArgs {}, arFlags {}{}",
+      showShort(target), numInitArgs, arFlags,
       formingRegion ? ",formingRegion" :""
     );
   }
 
   size_t stableHash() const {
     return folly::hash::hash_combine(
       SrcKey::StableHasher()(target),
-      std::hash<int32_t>()(spOffset.offset),
       std::hash<uint32_t>()(numInitArgs),
       std::hash<uint32_t>()(arFlags),
       std::hash<bool>()(formingRegion)
@@ -1352,7 +1349,6 @@ struct CallFuncEntryData : IRExtraData {
 
   bool equals(const CallFuncEntryData& o) const {
     return target == o.target &&
-           spOffset == o.spOffset &&
            numInitArgs == o.numInitArgs &&
            arFlags == o.arFlags &&
            formingRegion == o.formingRegion;
@@ -1362,12 +1358,7 @@ struct CallFuncEntryData : IRExtraData {
     return arFlags & (1 << ActRec::AsyncEagerRet);
   }
 
-  void updateStackOffsets(int32_t delta) {
-    spOffset.offset += delta;
-  }
-
   SrcKey target;
-  IRSPRelOffset spOffset;
   uint32_t numInitArgs;
   uint32_t arFlags;
   bool formingRegion;
@@ -3054,6 +3045,40 @@ struct LdClsFallbackData : IRExtraData {
   LdClsFallback fallback;
 };
 
+struct EnterInlineFrameData : IRExtraData {
+  EnterInlineFrameData(bool copy, uint32_t numInitArgs)
+    : copy(copy)
+    , numInitArgs(numInitArgs)
+  {}
+
+  static EnterInlineFrameData Copy(uint32_t numInitArgs) {
+    return EnterInlineFrameData {true, numInitArgs};
+  }
+
+  static EnterInlineFrameData NoCopy() {
+    return EnterInlineFrameData {false, 0};
+  }
+
+  std::string show() const {
+    return folly::to<std::string>(copy ? "copy, " : "",
+                                  "numInitArgs ", numInitArgs);
+  }
+  size_t hash() const {
+    return folly::hash::hash_combine(
+      copy ? 1 : 0,
+      std::hash<uint32_t>()(numInitArgs)
+    );
+  }
+  size_t stableHash() const { return hash(); }
+
+  bool equals(const EnterInlineFrameData& o) const {
+    return copy == o.copy && numInitArgs == o.numInitArgs;
+  }
+
+  bool copy;
+  uint32_t numInitArgs;
+};
+
 //////////////////////////////////////////////////////////////////////
 
 #define X(op, data)                                                   \
@@ -3134,6 +3159,7 @@ X(CallBuiltin,                  CallBuiltinData);
 X(CallFuncEntry,                CallFuncEntryData);
 X(InlineSideExit,               InlineSideExitData);
 X(InlineSideExitSyncStack,      StackRange);
+X(EnterInlineFrame,             EnterInlineFrameData);
 X(RetCtrl,                      RetCtrlData);
 X(AsyncFuncRet,                 IRSPRelOffsetData);
 X(AsyncFuncRetSlow,             IRSPRelOffsetData);

hphp/runtime/vm/jit/frame-state.cpp

@@ -347,20 +347,17 @@ void FrameStateMgr::update(const IRInstruction* inst) {
       assertx(cur().checkMInstrStateDead());
       auto const extra = inst->extra<CallFuncEntry>();
       auto const callee = extra->target.func();
-      // Remove tracked state for the slots for argc and the actrec.
-      uint32_t numCells = kNumActRecCells + callee->numFuncEntryInputs();
-      for (auto i = uint32_t{0}; i < numCells; ++i) {
-        setValue(stk(extra->spOffset + i), nullptr);
-      }
+      auto const defCalleeFP = inst->src(0)->inst();
+      assertx(defCalleeFP->is(DefCalleeFP));
+
       // Set the type of out parameter locations.
-      auto const base = extra->spOffset + numCells;
+      auto const base =
+        defCalleeFP->extra<DefCalleeFP>()->spOffset + kNumActRecCells;
       for (auto i = uint32_t{0}; i < callee->numInOutParams(); ++i) {
         setType(stk(base + i), irgen::callOutType(callee, i));
       }
       // We consider popping an ActRec and args to be synced to memory.
       assertx(cur().bcSPOff == inst->marker().bcSPOff());
-      assertx(cur().bcSPOff.offset >= numCells);
-      cur().bcSPOff -= numCells;
     }
     break;
 
@@ -498,16 +495,20 @@ void FrameStateMgr::update(const IRInstruction* inst) {
 
   case StLoc:
   case StLocMeta:
-    setValueAndSyncMBase(
-      loc(inst->extra<LocalId>()->locId),
-      inst->src(1),
-      false
-    );
+    if (inst->src(0) == cur().fp()) {
+      setValueAndSyncMBase(
+        loc(inst->extra<LocalId>()->locId),
+        inst->src(1),
+        false
+      );
+    }
     break;
 
   case LdLoc: {
-    auto const id = inst->extra<LdLoc>()->locId;
-    setValueAndSyncMBase(loc(id), inst->dst(), true);
+    if (inst->src(0) == cur().fp()) {
+      auto const id = inst->extra<LdLoc>()->locId;
+      setValueAndSyncMBase(loc(id), inst->dst(), true);
+    }
     break;
   }
 
@@ -1011,14 +1012,17 @@ void FrameStateMgr::trackEnterInlineFrame(const IRInstruction* inst) {
   auto const extra = inst->src(0)->inst()->extra<DefCalleeFP>();
   auto const callee = extra->func;
   auto const spOffset = extra->spOffset;
-  assertx(cur().bcSPOff == spOffset.to<SBInvOffset>(irSPOff()));
+  auto const adjust = inst->extra<EnterInlineFrame>()->copy
+    ? callee->numFuncEntryInputs()
+    : 0;
+  assertx(cur().bcSPOff - adjust == spOffset.to<SBInvOffset>(irSPOff()));
 
   // Remove space from callee's frame from the caller's stack.
-  for (auto i = uint32_t{0}; i < kNumActRecCells; ++i) {
-    setValue(stk(spOffset + i), nullptr);
+  for (auto i = uint32_t{0}; i < kNumActRecCells + adjust; ++i) {
+    setValue(stk(spOffset - adjust + i), nullptr);
   }
   assertx(cur().bcSPOff.offset >= kNumActRecCells);
-  cur().bcSPOff -= kNumActRecCells;
+  cur().bcSPOff -= kNumActRecCells + adjust;
 
   if (callee->isCPPBuiltin()) {
     auto const inout = callee->numInOutParams();

hphp/runtime/vm/jit/inlining-decider.cpp

@@ -725,10 +725,13 @@ RegionDescPtr selectCalleeCFG(SrcKey callerSk, SrcKey entry,
 }
 }
 
-irgen::RegionAndLazyUnit selectCalleeRegion(const irgen::IRGS& irgs,
-                                            SrcKey entry,
-                                            Type ctxType,
-                                            SrcKey callerSk) {
+irgen::RegionAndLazyUnit selectCalleeRegion(
+  const irgen::IRGS& irgs,
+  SrcKey entry,
+  Type ctxType,
+  SrcKey callerSk,
+  std::vector<Type> inputTypes
+) {
   assertx(entry.funcEntry());
   auto const callee = entry.func();
 
@@ -778,20 +781,13 @@ irgen::RegionAndLazyUnit selectCalleeRegion(const irgen::IRGS& irgs,
   }
 
   FTRACE(2, "selectCalleeRegion: callee = {}\n", callee->fullName()->data());
-  auto const firstInputPos = callee->numFuncEntryInputs() - 1;
-  std::vector<Type> inputTypes;
-  for (uint32_t i = 0; i < callee->numFuncEntryInputs(); ++i) {
-    // DataTypeGeneric is used because we're just passing the locals into the
-    // callee.  It's up to the callee to constrain further if needed.
-    auto const offset = BCSPRelOffset{safe_cast<int32_t>(firstInputPos - i)};
-    auto const type = irgen::publicTopType(irgs, offset);
-    assertx(type <= TCell);
+  assertx(inputTypes.size() == callee->numFuncEntryInputs());
 
+  for (int i = 0; i < inputTypes.size(); ++i) {
     // If we don't have sufficient type information to inline the region return
     // early
-    if (type == TBottom) return {callerSk, nullptr};
-    FTRACE(2, "input {}: {}\n", i + 1, type);
-    inputTypes.push_back(type);
+    if (inputTypes[i] == TBottom) return {callerSk, nullptr};
+    FTRACE(2, "input {}: {}\n", i + 1, inputTypes[i]);
   }
 
   while (entry.trivialDVFuncEntry()) {

hphp/runtime/vm/jit/inlining-decider.h

@@ -103,7 +103,8 @@ int costOfInlining(SrcKey callerSk,
 irgen::RegionAndLazyUnit selectCalleeRegion(const irgen::IRGS& irgs,
                                             SrcKey entry,
                                             Type ctxType,
-                                            SrcKey callerSk);
+                                            SrcKey callerSk,
+                                            std::vector<Type> inputTypes);
 
 void setInliningMetadata(uint64_t baseProfCount, 
                          const jit::hash_map<FuncId, uint32_t>& funcTargetCounts);