Don't spill to mmx.

There is not a clear path forward to safely using MMX registers as
scratch storage. Doing so spoils the state of the legacy x87 FPU, and the
x64 ABI uses the x87 FPU to implement long double. Our options are to
either:

  1. Prohibit use of long double in source code (or x87 instructions in
     machine code). Given that we have a dynamically linked binary that
     includes system libraries outside our control, open source that needs
     to be patched, fbcode, etc., this could prove difficult.

  2. Always execute an FPU resetting instruction before transitioning from
     TC code to C++. For all I know, these instructions are cheap, but it
     still seems like an unfortunate overhead to impose everywhere; since
     we don't want to mark every helper with a "reset fpu" call, we'd
     probably end up bloating callsites.

  3. Admit this doesn't work.

In the absence of evidence this matters for perf, I lean towards 3.

hphp/runtime/base/runtime_option.h

@@ -424,7 +424,6 @@ public:
   F(bool, HHIREnableCalleeSavedOpt,    true) \
   F(bool, HHIREnablePreColoring,       true) \
   F(bool, HHIREnableCoalescing,        true) \
-  F(bool, HHIREnableMmx,               false) \
   F(bool, HHIREnableRefCountOpt,       true) \
   F(bool, HHIREnableSinking,           true) \
   F(bool, HHIRGenerateAsserts,         debug) \

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

@@ -2198,16 +2198,9 @@ void CodeGenerator::cgSpill(IRInstruction* inst) {
 
     // We do not need to mask booleans, since the IR will reload the spill
     auto sinfo = dst->getSpillInfo(locIndex);
-    switch (sinfo.type()) {
-    case SpillInfo::MMX:
-      m_as.    mov_reg64_mmx(srcReg, sinfo.mmx());
-      break;
-    case SpillInfo::Memory:
-      m_as.    store_reg64_disp_reg64(srcReg,
-                                      sizeof(uint64_t) * sinfo.mem(),
-                                      reg::rsp);
-      break;
-    }
+    assert(sinfo.type() == SpillInfo::Memory);
+    m_as.    storeq(srcReg, reg::rsp[sizeof(uint64_t) * sinfo.mem()]);
+                                    
   }
 }
 
@@ -2220,16 +2213,8 @@ void CodeGenerator::cgReload(IRInstruction* inst) {
     auto dstReg = dst->getReg(locIndex);
 
     auto sinfo = src->getSpillInfo(locIndex);
-    switch (sinfo.type()) {
-    case SpillInfo::MMX:
-      m_as.    mov_mmx_reg64(sinfo.mmx(), dstReg);
-      break;
-    case SpillInfo::Memory:
-      m_as.    load_reg64_disp_reg64(reg::rsp,
-                                     sizeof(uint64_t) * sinfo.mem(),
-                                     dstReg);
-      break;
-    }
+    assert(sinfo.type() == SpillInfo::Memory);
+    m_as.    loadq(reg::rsp[sizeof(uint64_t) * sinfo.mem()], dstReg);
   }
 }
 

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

@@ -1723,18 +1723,14 @@ Type outputType(const IRInstruction*, int dstId = 0);
 void assertOperandTypes(const IRInstruction*);
 
 struct SpillInfo {
-  enum Type { MMX, Memory };
+  enum Type { Memory }; // Currently only one type of spill supported.
 
-  explicit SpillInfo(RegNumber r) : m_type(MMX), m_val(int(r)) {}
   explicit SpillInfo(uint32_t v)  : m_type(Memory), m_val(v) {}
 
   Type      type() const { return m_type; }
 
-  // return MMX register number for this spill
-  RegNumber mmx()  const { return RegNumber(m_val); }
-
   // return offset in 8-byte-words from stack pointer
-  uint32_t  mem()  const { return m_val; }
+  uint32_t  mem()  const { assert(m_type == Memory); return m_val; }
 
 private:
   Type     m_type : 1;
@@ -1743,9 +1739,6 @@ private:
 
 inline std::ostream& operator<<(std::ostream& os, SpillInfo si) {
   switch (si.type()) {
-  case SpillInfo::MMX:
-    os << "mmx" << reg::regname(RegXMM(int(si.mmx())));
-    break;
   case SpillInfo::Memory:
     os << "spill[" << si.mem() << "]";
     break;

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

@@ -27,8 +27,6 @@ using namespace Transl::reg;
 
 static const HPHP::Trace::Module TRACEMOD = HPHP::Trace::hhir;
 
-const int NumMmxRegs = 8;
-
 struct LinearScan : private boost::noncopyable {
   static const int NumRegs = 16;
 
@@ -440,13 +438,12 @@ void LinearScan::allocRegToTmp(RegState* reg, SSATmp* ssaTmp, uint32_t index) {
 // Assign spill location numbers to Spill/Reload.
 uint32_t LinearScan::assignSpillLoc() {
   uint32_t nextSpillLoc = 0;
-  uint32_t nextMmxReg = 0;
 
   // visit blocks in reverse postorder and instructions in forward order,
-  // assigning a spill slot id or mmx register number to each Spill.
-  // We don't reuse slot id's or mmx registers, but both could be reused
-  // either by visiting the dominator tree in preorder or by analyzing
-  // lifetimes and reusing id/registers between non-conflicting spills.
+  // assigning a spill slot id to each Spill. We don't reuse slot id's,
+  // but both could be reused either by visiting the dominator tree in
+  // preorder or by analyzing lifetimes and reusing id/registers between
+  // non-conflicting spills.
 
   for (Block* block : m_blocks) {
     for (IRInstruction& inst : *block) {
@@ -466,22 +463,8 @@ uint32_t LinearScan::assignSpillLoc() {
             TRACE(3, "[counter] 1 spill a tmp that spans native\n");
           }
 
-          const bool allowMmxSpill = RuntimeOption::EvalHHIREnableMmx &&
-            // The live range of the spill slot doesn't span native calls,
-            // and we still have free MMX registers.
-            dst->getLastUseId() <= getNextNativeId() &&
-            nextMmxReg < (uint32_t)NumMmxRegs;
-
-          dst->setSpillInfo(locIndex,
-            allowMmxSpill
-              ? SpillInfo(RegNumber(nextMmxReg++))
-              : SpillInfo(nextSpillLoc++)
-          );
-          if (allowMmxSpill) {
-            TRACE(3, "[counter] 1 spill to mmx\n");
-          } else {
-            TRACE(3, "[counter] 1 spill to memory\n");
-          }
+          dst->setSpillInfo(locIndex, SpillInfo(nextSpillLoc++));
+          TRACE(3, "[counter] 1 spill\n");
         }
       }
       if (inst.getOpcode() == Reload) {
@@ -489,11 +472,7 @@ uint32_t LinearScan::assignSpillLoc() {
         for (int locIndex = 0;
              locIndex < src->numNeededRegs();
              ++locIndex) {
-          if (src->getSpillInfo(locIndex).type() == SpillInfo::MMX) {
-            TRACE(3, "[counter] reload from mmx\n");
-          } else {
-            TRACE(3, "[counter] reload from memory\n");
-          }
+          TRACE(3, "[counter] reload\n");
         }
       }
     }
@@ -816,9 +795,6 @@ void LinearScan::allocRegs(Trace* trace) {
     rematerialize();
   }
 
-  // assignSpillLoc needs next natives in order to decide whether we
-  // can use MMX registers.
-  collectNatives();
   // Make sure rsp is 16-aligned.
   uint32_t numSpillLocs = assignSpillLoc();
   if (numSpillLocs % 2) {