Move m_liveRegs out of IRInstruction

The live regs field in IRInstruction is the result of an analysis
pass which can be better encapsulated by moving the field into
its own StateVector.

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

@@ -810,13 +810,25 @@ void CodeGenerator::cgCallHelper(Asm& a,
                                  SyncOptions sync,
                                  ArgGroup& args,
                                  DestType destType) {
+  cgCallHelper(a, call, dstReg0, dstReg1, sync, args,
+               m_state.liveRegs[m_curInst], destType);
+}
+
+void CodeGenerator::cgCallHelper(Asm& a,
+                                 const Transl::Call& call,
+                                 PhysReg dstReg0,
+                                 PhysReg dstReg1,
+                                 SyncOptions sync,
+                                 ArgGroup& args,
+                                 RegSet toSave,
+                                 DestType destType) {
   assert(int(args.size()) <= kNumRegisterArgs);
   assert(m_curInst->isNative());
 
   // Save the register that are live at the point after this IR instruction.
   // However, don't save the destination registers that will be overwritten
   // by this call.
-  RegSet toSave = m_curInst->getLiveRegs() & kCallerSaved;
+  toSave = toSave & kCallerSaved;
   assert((toSave & RegSet().add(dstReg0).add(dstReg1)).empty());
   PhysRegSaverParity<1> regSaver(a, toSave);
 
@@ -2684,7 +2696,7 @@ void CodeGenerator::cgGenericRetDecRefs(IRInstruction* inst) {
    * are still allocated to registers at this time.
    */
   const auto UNUSED expectedLiveRegs = RegSet(rFp).add(rDest) | retvalRegs;
-  assert((m_curInst->getLiveRegs() - expectedLiveRegs).empty());
+  assert((m_state.liveRegs[m_curInst] - expectedLiveRegs).empty());
   assert(rFp == rVmFp &&
          "free locals helper assumes the frame pointer is rVmFp");
   assert(rDest == rVmSp &&
@@ -2999,7 +3011,7 @@ void CodeGenerator::cgDecRefDynamicTypeMem(PhysReg baseReg,
   if (exit == nullptr && RuntimeOption::EvalHHIRGenericDtorHelper) {
     {
       // This PhysRegSaverParity saves rdi redundantly if
-      // !m_curInst->getLiveRegs().contains(rdi), but its
+      // !liveRegs[m_curInst].contains(rdi), but its
       // necessary to maintain stack alignment. We can do better
       // by making the helpers adjust the stack for us in the cold
       // path, which calls the destructor.
@@ -4256,18 +4268,17 @@ void CodeGenerator::cgLdClsMethodFCache(IRInstruction* inst) {
 
   // Handle case where method is not entered in the cache
   unlikelyIfBlock(CC_E, [&] (Asm& a) {
-    if (false) { // typecheck
-      const UNUSED Func* f = StaticMethodFCache::lookupIR(ch, cls, methName);
-    }
+    const Func* (*lookup)(CacheHandle, const Class*, const StringData*) =
+      StaticMethodFCache::lookupIR;
     // preserve destCtxReg across the call since it wouldn't be otherwise
-    inst->setLiveRegs(inst->getLiveRegs().add(destCtxReg));
-    cgCallHelper(a,
-                 (TCA)StaticMethodFCache::lookupIR,
-                 funcDestReg,
+    RegSet toSave = m_state.liveRegs[inst] | RegSet(destCtxReg);
+    cgCallHelper(a, Transl::Call((TCA)lookup),
+                 funcDestReg, InvalidReg,
                  kSyncPoint,
                  ArgGroup().imm(ch)
                            .immPtr(cls)
-                           .immPtr(methName));
+                           .immPtr(methName),
+                 toSave);
     // If entry found in target cache, jump back to m_as.
     // Otherwise, bail to exit label
     a.testq(funcDestReg, funcDestReg);
@@ -5146,6 +5157,35 @@ void CodeGenerator::print() const {
   JIT::print(std::cout, m_curTrace, m_state.asmInfo);
 }
 
+/*
+ * Compute and save registers that are live *across* each inst, not including
+ * registers whose lifetimes end at inst, nor registers defined by inst.
+ */
+LiveRegs computeLiveRegs(const IRFactory* factory, Block* start_block) {
+  StateVector<Block, RegSet> liveMap(factory, RegSet());
+  LiveRegs live_regs(factory, RegSet());
+  postorderWalk(
+    [&](Block* block) {
+      RegSet& live = liveMap[block];
+      if (Block* taken = block->getTaken()) live = liveMap[taken];
+      if (Block* next = block->getNext()) live |= liveMap[next];
+      for (auto it = block->end(); it != block->begin(); ) {
+        IRInstruction& inst = *--it;
+        for (const SSATmp& dst : inst.getDsts()) {
+          live -= dst.getRegs();
+        }
+        live_regs[inst] = live;
+        for (SSATmp* src : inst.getSrcs()) {
+          live |= src->getRegs();
+        }
+      }
+    },
+    factory->numBlocks(),
+    start_block
+  );
+  return live_regs;
+}
+
 // select instructions for the trace and its exits
 void genCodeForTrace(Trace* trace,
                      CodeGenerator::Asm& as,
@@ -5155,7 +5195,8 @@ void genCodeForTrace(Trace* trace,
                      Transl::TranslatorX64* tx64,
                      AsmInfo* asmInfo) {
   assert(trace->isMain());
-  CodegenState state(irFactory, asmInfo);
+  LiveRegs live_regs = computeLiveRegs(irFactory, trace->front());
+  CodegenState state(irFactory, live_regs, asmInfo);
   cgTrace(trace, as, astubs, tx64, bcMap, state);
   for (Trace* exit : trace->getExitTraces()) {
     cgTrace(exit, astubs, astubs, tx64, nullptr, state);

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

@@ -67,12 +67,16 @@ struct AsmInfo {
   StateVector<Block,TcaRange> astubRanges;
 };
 
+typedef StateVector<IRInstruction, RegSet> LiveRegs;
+
 // Stuff we need to preserve between blocks while generating code,
 // and address information produced during codegen.
 struct CodegenState {
-  CodegenState(const IRFactory* factory, AsmInfo* asmInfo)
+  CodegenState(const IRFactory* factory, const LiveRegs& liveRegs,
+               AsmInfo* asmInfo)
     : patches(factory, nullptr)
     , lastMarker(nullptr)
+    , liveRegs(liveRegs)
     , asmInfo(asmInfo)
   {}
 
@@ -87,6 +91,12 @@ struct CodegenState {
   // next block in the same assmbler.
   bool noTerminalJmp_;
 
+  // for each instruction, holds the RegSet of registers that must be
+  // preserved across that instruction.  This is for push/pop of caller-saved
+  // registers.
+  const LiveRegs& liveRegs;
+
+  // Output: start/end ranges of machine code addresses of each instruction.
   AsmInfo* asmInfo;
 };
 
@@ -143,6 +153,14 @@ private:
                     SyncOptions sync,
                     ArgGroup& args,
                     DestType destType = DestType::SSA);
+  void cgCallHelper(Asm& a,
+                    const Transl::Call& call,
+                    PhysReg dstReg0,
+                    PhysReg dstReg1,
+                    SyncOptions sync,
+                    ArgGroup& args,
+                    RegSet toSave,
+                    DestType destType = DestType::SSA);
 
   void cgStore(PhysReg base,
                int64_t off,

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

@@ -599,7 +599,6 @@ void IRInstruction::convertToNop() {
   m_numSrcs = nop.m_numSrcs;
   m_id = nop.m_id;
   m_srcs = nop.m_srcs;
-  m_liveRegs = nop.m_liveRegs;
   m_numDsts = nop.m_numDsts;
   m_dst = nop.m_dst;
   m_taken = nullptr;
@@ -637,7 +636,6 @@ void IRInstruction::become(IRFactory* factory, IRInstruction* other) {
   m_op = other->m_op;
   m_typeParam = other->m_typeParam;
   m_id = other->m_id;
-  m_liveRegs = other->m_liveRegs;
   m_taken = other->m_taken;
   m_tca = other->m_tca;
   m_numSrcs = other->m_numSrcs;

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

@@ -1852,11 +1852,6 @@ struct IRInstruction {
    */
   bool       isTransient() const       { return m_iid == kTransient; }
 
-  // LiveRegs is the set of registers that are live across this instruction.
-  // Doesn't include dest registers, or src registers whose lifetime ends here.
-  RegSet     getLiveRegs() const       { return m_liveRegs; }
-  void       setLiveRegs(RegSet s)     { m_liveRegs = s; }
-
   Block*     getBlock() const          { return m_block; }
   void       setBlock(Block* b)        { m_block = b; }
   Trace*     getTrace() const;
@@ -1919,7 +1914,6 @@ private:
   const IId         m_iid;
   uint32_t          m_id;
   SSATmp**          m_srcs;
-  RegSet            m_liveRegs;
   SSATmp*           m_dst;     // if HasDest or NaryDest
   Block*            m_taken;   // for branches, guards, and jmp
   Block*            m_block;   // block that owns this instruction
@@ -2319,7 +2313,7 @@ struct Block : boost::noncopyable {
   // list-compatible interface; these delegate to m_instrs but also update
   // inst.m_block
   InstructionList& getInstrs()   { return m_instrs; }
-  bool             empty()       { return m_instrs.empty(); }
+  bool             empty() const { return m_instrs.empty(); }
   iterator         begin()       { return m_instrs.begin(); }
   iterator         end()         { return m_instrs.end(); }
   const_iterator   begin() const { return m_instrs.begin(); }

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

@@ -113,7 +113,6 @@ private:
   void allocRegToTmp(SSATmp* ssaTmp, uint32_t index);
   void freeRegsAtId(uint32_t id);
   void spill(SSATmp* tmp);
-  void computeLiveRegs();
 
   template<typename T> SSATmp* cns(T val) {
     return m_irFactory->cns(val);
@@ -256,32 +255,6 @@ LinearScan::LinearScan(IRFactory* irFactory)
   }
 }
 
-/*
- * Compute and save registers that are live *across* each inst, not including
- * registers whose lifetimes end at inst, nor registers defined by inst.
- */
-void LinearScan::computeLiveRegs() {
-  StateVector<Block, RegSet> liveMap(m_irFactory, RegSet());
-  postorderWalk(
-    [&](Block* block) {
-      RegSet& live = liveMap[block];
-      if (Block* taken = block->getTaken()) live = liveMap[taken];
-      if (Block* next = block->getNext()) live |= liveMap[next];
-      for (auto it = block->end(); it != block->begin(); ) {
-        IRInstruction& inst = *--it;
-        for (const SSATmp& dst : inst.getDsts()) {
-          RegSet d = dst.getRegs();
-          live -= d;
-        }
-        inst.setLiveRegs(live);
-        for (SSATmp* src : inst.getSrcs()) {
-          live |= src->getRegs();
-        }
-      }
-    },
-    m_irFactory->numBlocks(), m_blocks.front());
-}
-
 template<typename Inner, int DumpVal=4>
 static inline void dumpIR(const Inner* in, const char* msg) {
   if (dumpIREnabled(DumpVal)) {
@@ -1033,9 +1006,6 @@ void LinearScan::allocRegs(Trace* trace) {
   }
 
   if (m_slots.size()) genSpillStats(trace, numSpillLocs);
-
-  // record the live register set at each instruction
-  computeLiveRegs();
 }
 
 void LinearScan::allocRegsOneTrace(BlockList::iterator& blockIt,