Access m_type via helpers.

We had the belief that m_type as an int32_t (and in at least one
place, an int64_t) burned in many places. This is going to make any kind of
re-encoding of TypedValues nearly impossible.

Redirect all such accesses via some helpers, so e.g.

        a.  cmpl(KindOfUninit, base[TVOFF(m_type)]);

becomes

       emitCmpTVType(a, KindOfUninit, base[TVOFF(m_type)]);

which may do byte or dword access, depending on m_type's actual size. While
this is motivated by 7pack, I'm planning to route it through trunk to
prevent any more of the old style accesses from cropping up.

hphp/runtime/base/types.h

@@ -162,7 +162,7 @@ enum DataType {
   MaxNumDataTypes        = KindOfIndirect + 1, // marker, not a valid type
   MaxNumDataTypesIndex   = 11 + 1,  // 1 + the number of valid DataTypes above
 
-  MaxDataType            = 0x7fffffff, // Allow KindOf* > 11 in HphpArray.
+  MaxDataType            = 0x7f, // Allow KindOf* > 11 in HphpArray.
 
   // Note: KindOfStringBit must be set in KindOfStaticString and KindOfString,
   //       and it must be 0 in any other real DataType.
@@ -172,8 +172,6 @@ enum DataType {
   //       and StaticString, and it must be 0 for any other real DataType.
   KindOfUncountedInitBit = 8,
 };
-BOOST_STATIC_ASSERT((sizeof(DataType) == 4));
-
 
 BOOST_STATIC_ASSERT(KindOfString       & KindOfStringBit);
 BOOST_STATIC_ASSERT(KindOfStaticString & KindOfStringBit);

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

@@ -35,6 +35,7 @@
 #include "runtime/vm/translator/targetcache.h"
 #include "runtime/vm/translator/translator-inline.h"
 #include "runtime/vm/translator/translator-x64.h"
+#include "runtime/vm/translator/translator-x64-internal.h"
 #include "runtime/vm/translator/translator.h"
 #include "runtime/vm/translator/types.h"
 #include "runtime/vm/translator/x64-util.h"
@@ -65,7 +66,7 @@ emitDeref(X64Assembler &a, PhysReg src, PhysReg dest) {
 
 /* static */ void
 emitDerefIfVariant(X64Assembler &a, PhysReg reg) {
-  a.cmp_imm32_disp_reg32(HPHP::KindOfRef, TVOFF(m_type), reg);
+  emitCmpTVType(a, HPHP::KindOfRef, reg[TVOFF(m_type)]);
   a.cload_reg64_disp_reg64(CC_Z, reg, 0, reg);
 }
 
@@ -1327,16 +1328,16 @@ ConditionCode CodeGenerator::emitTypeTest(Type type, OpndType src,
   assert(!type.subtypeOf(Type::Cls));
 
   if (type.isString()) {
-    m_as.testl(KindOfStringBit, src);
+    emitTestTVType(m_as, KindOfStringBit, src);
     cc = CC_NZ;
   } else if (type.equals(Type::UncountedInit)) {
-    m_as.testl(KindOfUncountedInitBit, src);
+    emitTestTVType(m_as, KindOfUncountedInitBit, src);
     cc = CC_NZ;
   } else if (type.equals(Type::Uncounted)) {
-    m_as.cmpl(KindOfRefCountThreshold, src);
+    emitCmpTVType(m_as, KindOfRefCountThreshold, src);
     cc = CC_LE;
   } else if (type.equals(Type::Cell)) {
-    m_as.cmpl(KindOfRef, src);
+    emitCmpTVType(m_as, KindOfRef, src);
     cc = CC_L;
   } else if (type.equals(Type::Gen)) {
     return CC_None; // nothing to check
@@ -1344,8 +1345,8 @@ ConditionCode CodeGenerator::emitTypeTest(Type type, OpndType src,
     DataType dataType = type.toDataType();
     assert(dataType == KindOfRef ||
            (dataType >= KindOfUninit && dataType <= KindOfObject));
-    m_as.cmpl(dataType, src);
-    cc = CC_Z;
+    emitCmpTVType(m_as, dataType, src);
+    cc = CC_E;
   }
   return negate ? ccNegate(cc) : cc;
 }
@@ -1765,11 +1766,20 @@ void CodeGenerator::cgUnbox(IRInstruction* inst) {
   PhysReg tmpDstTypeReg = srcValReg == dstTypeReg ? PhysReg(rScratch)
                                                   : dstTypeReg;
 
-  m_as.   cmpq(HPHP::KindOfRef, srcTypeReg);
+  bool useCMov = sizeof(DataType) == 4;
   emitMovRegReg(m_as, srcTypeReg, tmpDstTypeReg);
-  m_as.   cload_reg64_disp_reg64(CC_Z, srcValReg, TVOFF(m_type), tmpDstTypeReg);
   emitMovRegReg(m_as, srcValReg, dstValReg);
-  m_as.   cload_reg64_disp_reg64(CC_Z, srcValReg, TVOFF(m_data), dstValReg);
+  emitCmpTVType(m_as, HPHP::KindOfRef, srcTypeReg);
+  // XXX: hardcodes RefData <-> TypedValue pun.
+  if (useCMov) {
+    m_as.  cload_reg64_disp_reg32(CC_Z, srcValReg, TVOFF(m_type), tmpDstTypeReg);
+    m_as.  cload_reg64_disp_reg64(CC_Z, srcValReg, TVOFF(m_data), dstValReg);
+  } else {
+    ifThen(m_as, CC_E, [&]() {
+      emitLoadTVType(m_as, srcValReg[TVOFF(m_type)], tmpDstTypeReg);
+      m_as.loadq(srcValReg[TVOFF(m_data)], dstValReg);
+    });
+  }
   emitMovRegReg(m_as, tmpDstTypeReg, dstTypeReg);
 }
 
@@ -1829,10 +1839,10 @@ void CodeGenerator::cgRetVal(IRInstruction* inst) {
   // Store return value at the top of the caller's eval stack
   // (a) Store the type
   if (val->getType().needsReg()) {
-    a.    storel (r32(val->getReg(1)), rFp[AROFF(m_r) + TVOFF(m_type)]);
+    emitStoreTVType(a, val->getReg(1), rFp[AROFF(m_r) + TVOFF(m_type)]);
   } else {
-    a.    storel (val->getType().toDataType(),
-                  rFp[AROFF(m_r) + TVOFF(m_type)]);
+    emitStoreTVType(a, val->getType().toDataType(),
+                    rFp[AROFF(m_r) + TVOFF(m_type)]);
   }
 
   // (b) Store the actual value (not necessary when storing Null)
@@ -2682,11 +2692,7 @@ Address CodeGenerator::cgCheckRefCountedType(PhysReg typeReg) {
 
 Address CodeGenerator::cgCheckRefCountedType(PhysReg baseReg,
                                              int64_t offset) {
-
-  m_as.cmp_imm32_disp_reg32(KindOfRefCountThreshold,
-                            offset + TVOFF(m_type),
-                            baseReg);
-
+  emitCmpTVType(m_as, KindOfRefCountThreshold, baseReg[offset + TVOFF(m_type)]);
   Address addrToPatch =  m_as.code.frontier;
   m_as.jcc8(CC_LE, addrToPatch);
 
@@ -3164,7 +3170,7 @@ void CodeGenerator::cgCallBuiltin(IRInstruction* inst) {
   }
   if (returnType.subtypeOf(Type::Cell)
       || returnType.subtypeOf(Type::BoxedCell)) {
-    m_as.   loadl (returnBase[returnOffset + TVOFF(m_type)], r32(dstType));
+    emitLoadTVType(m_as, returnBase[returnOffset + TVOFF(m_type)], dstType);
     m_as.   loadq (returnBase[returnOffset + TVOFF(m_data)], dstReg);
     emitLoadImm(m_as, KindOfNull, rScratch);
     static_assert(KindOfUninit == 0,
@@ -3382,35 +3388,25 @@ void CodeGenerator::cgStRaw(IRInstruction* inst) {
   RawMemSlot& slot = RawMemSlot::Get(RawMemSlot::Kind(kind));
   int stSize = slot.getSize();
   int64_t off = slot.getOffset();
+  auto dest = baseReg[off];
 
   if (value->isConst()) {
     if (stSize == sz::qword) {
-      m_as.store_imm64_disp_reg64(value->getValInt(),
-                                  off,
-                                  baseReg);
+      m_as.storeq(value->getValInt(), dest);
     } else if (stSize == sz::dword) {
-      m_as.store_imm32_disp_reg(value->getValInt(),
-                                  off,
-                                  baseReg);
+      m_as.storel(value->getValInt(), dest);
     } else {
       assert(stSize == sz::byte);
-      m_as.store_imm8_disp_reg(value->getValBool(),
-                               off,
-                               baseReg);
+      m_as.storeb(value->getValBool(), dest);
     }
   } else {
     if (stSize == sz::qword) {
-      m_as.store_reg64_disp_reg64(value->getReg(),
-                                  off,
-                                  baseReg);
+      m_as.storeq(r64(value->getReg()), dest);
     } else if (stSize == sz::dword) {
-      m_as.store_reg32_disp_reg64(value->getReg(),
-                                  off,
-                                  baseReg);
+      m_as.storel(r32(value->getReg()), dest);
     } else {
-      // not supported by our assembler yet
       assert(stSize == sz::byte);
-      not_implemented();
+      m_as.storeb(rbyte(value->getReg()), dest);
     }
   }
 }
@@ -3441,7 +3437,7 @@ void CodeGenerator::cgLoadTypedValue(PhysReg base,
 
   // Load type if it's not dead
   if (typeDstReg != InvalidReg) {
-    m_as.load_reg64_disp_reg32(base, off + TVOFF(m_type), typeDstReg);
+    emitLoadTVType(m_as, base[off + TVOFF(m_type)], typeDstReg);
     if (label) {
       // Check type needed
       emitGuardType(r32(typeDstReg), inst);
@@ -3468,10 +3464,7 @@ void CodeGenerator::cgStoreTypedValue(PhysReg base,
   m_as.store_reg64_disp_reg64(src->getReg(0),
                               off + TVOFF(m_data),
                               base);
-  // store the type
-  m_as.store_reg32_disp_reg64(src->getReg(1),
-                              off + TVOFF(m_type),
-                              base);
+  emitStoreTVType(m_as, src->getReg(1), base[off + TVOFF(m_type)]);
 }
 
 void CodeGenerator::cgStore(PhysReg base,
@@ -3485,9 +3478,7 @@ void CodeGenerator::cgStore(PhysReg base,
   }
   // store the type
   if (genStoreType) {
-    m_as.store_imm32_disp_reg(type.toDataType(),
-                              off + TVOFF(m_type),
-                              base);
+    emitStoreTVType(m_as, type.toDataType(), base[off + TVOFF(m_type)]);
   }
   if (type.isNull()) {
     // no need to store a value for null or uninit
@@ -4488,7 +4479,7 @@ void CodeGenerator::cgFillContThis(IRInstruction* inst) {
   ifThen(m_as, CC_NZ, [&] {
     m_as.addl(1, scratch[FAST_REFCOUNT_OFFSET]);
     m_as.storeq(scratch, baseReg[offset + TVOFF(m_data)]);
-    m_as.storel(KindOfObject, baseReg[offset + TVOFF(m_type)]);
+    emitStoreTVType(m_as, KindOfObject, baseReg[offset + TVOFF(m_type)]);
   });
 }
 

hphp/runtime/vm/translator/translator-x64-internal.h

@@ -22,12 +22,6 @@
 
 #include <runtime/vm/translator/abi-x64.h>
 
-/*
- * Please don't include this unless your file implements methods of
- * TranslatorX64; you won't like it. It pollutes the namespace, makes
- * "KindOfString" #error, makes your TRACEMOD tx64, and tortures a kitten.
- */
-
 using namespace HPHP::VM::Transl::reg;
 using namespace HPHP::Util;
 using namespace HPHP::Trace;
@@ -39,10 +33,6 @@ namespace Transl {
 
 static const Trace::Module TRACEMOD = Trace::tx64;
 static const DataType BitwiseKindOfString = KindOfString;
-
-#define KindOfString \
-#error You probably do not mean to use KindOfString in this file.
-
 // RAII aids to machine code.
 
 // In shared stubs, we've already made the stack odd by calling
@@ -581,7 +571,8 @@ asm_label(a, likely);
 // a ref-counted cell.
 //
 // It's ok to do reconcilable register operations in the body.
-template<int FieldOffset, int FieldValue, ConditionCode Jcc>
+template<int FieldOffset, int FieldValue, ConditionCode Jcc,
+         typename FieldType>
 struct CondBlock {
   X64Assembler& m_a;
   int m_off;
@@ -591,7 +582,13 @@ struct CondBlock {
   CondBlock(X64Assembler& a, PhysReg reg, int offset = 0)
       : m_a(a), m_off(offset), m_dg(new DiamondGuard(a)) {
     int typeDisp = m_off + FieldOffset;
-    a.   cmp_imm32_disp_reg32(FieldValue, typeDisp, reg);
+    static_assert(sizeof(FieldType) == 1 || sizeof(FieldType) == 4,
+                  "CondBlock of unimplemented field size");
+    if (sizeof(FieldType) == 4) {
+      a. cmpl(FieldValue, reg[typeDisp]);
+    } else if (sizeof(FieldType) == 1) {
+      a. cmpb(FieldValue, reg[typeDisp]);
+    }
     m_jcc8 = a.code.frontier;
     a.   jcc8(Jcc, m_jcc8);
     // ...
@@ -607,15 +604,18 @@ struct CondBlock {
 //   Emits if (IS_REFCOUNTED_TYPE()) { ... }
 typedef CondBlock <TVOFF(m_type),
                    KindOfRefCountThreshold,
-                   CC_LE> IfRefCounted;
+                   CC_LE,
+                   DataType> IfRefCounted;
 
 typedef CondBlock <TVOFF(m_type),
                    KindOfRef,
-                   CC_NZ> IfVariant;
+                   CC_NZ,
+                   DataType> IfVariant;
 
 typedef CondBlock <TVOFF(m_type),
                    KindOfUninit,
-                   CC_Z> UnlessUninit;
+                   CC_Z,
+                   DataType> UnlessUninit;
 
 /*
  * locToRegDisp --
@@ -706,6 +706,76 @@ local_name(const Location& l) {
   return ret;
 }
 
+static_assert(sizeof(DataType) == 4 || sizeof(DataType) == 1,
+              "Your DataType has an unsupported size.");
+static inline Reg8 toByte(const Reg32& x)   { return rbyte(x); }
+static inline Reg8 toByte(const Reg64& x)   { return rbyte(x); }
+static inline Reg8 toByte(PhysReg x)        { return rbyte(x); }
+
+static inline Reg32 toReg32(const Reg64& x) { return r32(x); }
+static inline Reg32 toReg32(PhysReg x)      { return r32(x); }
+
+// For other operand types, let whatever conversions (or compile
+// errors) exist handle it.
+template<typename OpndType>
+static OpndType toByte(const OpndType& x) { return x; }
+template<typename OpndType>
+static OpndType toReg32(const OpndType& x) { return x; }
+
+template<typename OpndType>
+static inline void verifyTVOff(const OpndType& op) { /* nop */ }
+static inline void verifyTVOff(const MemoryRef& mr) {
+  DEBUG_ONLY auto disp = mr.r.disp;
+  // Make sure that we're operating on the m_type field of a
+  // TypedValue*.
+  assert((disp & (sizeof(TypedValue) - 1)) == TVOFF(m_type));
+}
+
+template<typename SrcType, typename OpndType>
+static inline void
+emitTestTVType(X64Assembler& a, SrcType src, OpndType tvOp) {
+  verifyTVOff(src);
+  if (sizeof(DataType) == 4) {
+    a.  testl(src, toReg32(tvOp));
+  } else {
+    a.  testb(src, toByte(tvOp));
+  }
+}
+
+template<typename SrcType, typename OpndType>
+static inline void
+emitLoadTVType(X64Assembler& a, SrcType src, OpndType tvOp) {
+  verifyTVOff(src);
+  if (sizeof(DataType) == 4) {
+    a.  loadl(src, toReg32(tvOp));
+  } else {
+    // Zero extend the type, just in case.
+    a.  loadzbl(src, toReg32(tvOp));
+  }
+}
+
+template<typename SrcType, typename OpndType>
+static inline void
+emitCmpTVType(X64Assembler& a, SrcType src, OpndType tvOp) {
+  verifyTVOff(src);
+  if (sizeof(DataType) == 4) {
+    a.  cmpl(src, toReg32(tvOp));
+  } else {
+    a.  cmpb(src, toByte(tvOp));
+  }
+}
+
+template<typename DestType, typename OpndType>
+static inline void
+emitStoreTVType(X64Assembler& a, OpndType tvOp, DestType dest) {
+  verifyTVOff(dest);
+  if (sizeof(DataType) == 4) {
+    a.  storel(toReg32(tvOp), dest);
+  } else {
+    a.  storeb(toByte(tvOp), dest);
+  }
+}
+
 // emitDispDeref --
 // emitDeref --
 // emitStoreTypedValue --
@@ -716,19 +786,19 @@ local_name(const Location& l) {
 //
 //   Dereference the var in the cell whose address lives in src into
 //   dest.
-static void
+static inline void
 emitDispDeref(X64Assembler &a, Reg64 src, int disp, Reg64 dest) {
   a.    loadq (src[disp + TVOFF(m_data)], dest);
 }
 
-static void
+static inline void
 emitDeref(X64Assembler &a, PhysReg src, PhysReg dest) {
   emitDispDeref(a, src, 0, dest);
 }
 
-static void
+static inline void
 emitDerefIfVariant(X64Assembler &a, PhysReg reg) {
-  a.cmp_imm32_disp_reg32(KindOfRef, TVOFF(m_type), reg);
+  emitCmpTVType(a, KindOfRef, reg[TVOFF(m_type)]);
   a.cload_reg64_disp_reg64(CC_Z, reg, TVOFF(m_data), reg);
 }
 
@@ -738,7 +808,7 @@ static inline void
 emitStoreTypedValue(X64Assembler& a, DataType type, PhysReg val,
                     int disp, PhysReg dest, bool writeType = true) {
   if (writeType) {
-    a.  store_imm32_disp_reg(type, disp + TVOFF(m_type), dest);
+    emitStoreTVType(a, type, dest[disp + TVOFF(m_type)]);
   }
   if (!IS_NULL_TYPE(type)) {
     assert(val != reg::noreg);
@@ -752,7 +822,7 @@ emitStoreInvalid(X64Assembler& a, int disp, PhysReg dest) {
                 "emitStoreInvalid assumes m_aux is dword sized.");
   a.    storeq (0xfacefacefaceface,     dest[disp + TVOFF(m_data)]);
   a.    storel ((signed int)0xfaceface, dest[disp + TypedValueAux::auxOffset]);
-  a.    storel (KindOfInvalid,          dest[disp + TVOFF(m_type)]);
+  emitStoreTVType(a, KindOfInvalid,     dest[disp + TVOFF(m_type)]);
 }
 
 static inline void
@@ -760,14 +830,14 @@ emitStoreUninitNull(X64Assembler& a,
                     int disp,
                     PhysReg dest) {
   // OK to leave garbage in m_data, m_aux.
-  a.    store_imm32_disp_reg(KindOfUninit, disp + TVOFF(m_type), dest);
+  emitStoreTVType(a, KindOfUninit, dest[disp + TVOFF(m_type)]);
 }
 
 static inline void
 emitStoreNull(X64Assembler& a,
               int disp,
               PhysReg dest) {
-  a.    store_imm32_disp_reg(KindOfNull, disp + TVOFF(m_type), dest);
+  emitStoreTVType(a, KindOfNull, dest[disp + TVOFF(m_type)]);
   // It's ok to leave garbage in m_data, m_aux for KindOfNull.
 }
 
@@ -792,8 +862,8 @@ emitCopyTo(X64Assembler& a,
   auto s32 = r32(scratch);
   a.    loadq  (src[srcOff + TVOFF(m_data)], s64);
   a.    storeq (s64, dest[destOff + TVOFF(m_data)]);
-  a.    loadl  (src[srcOff + TVOFF(m_type)], s32);
-  a.    storel (s32, dest[destOff + TVOFF(m_type)]);
+  emitLoadTVType(a, src[srcOff + TVOFF(m_type)], s32);
+  emitStoreTVType(a, s32, dest[destOff + TVOFF(m_type)]);
 }
 
 /*

hphp/runtime/vm/translator/translator-x64-vector.cpp

@@ -1003,7 +1003,7 @@ void TranslatorX64::emitPropSpecialized(MInstrAttr const mia,
   }
   a.    lea_reg64_disp_reg64(r(rBase), propOffset, r(rScratch));
   if (doWarn || doDefine) {
-    a.  cmp_imm32_disp_reg32(KindOfUninit, TVOFF(m_type), r(rScratch));
+    emitCmpTVType(a, KindOfUninit, r(rScratch)[TVOFF(m_type)]);
     {
       UnlikelyIfBlock ifUninit(CC_Z, a, astubs);
       if (doWarn) {
@@ -1014,7 +1014,7 @@ void TranslatorX64::emitPropSpecialized(MInstrAttr const mia,
         );
       }
       if (doDefine) {
-        astubs.store_imm32_disp_reg(KindOfNull, TVOFF(m_type), r(rScratch));
+        emitStoreTVType(astubs, KindOfNull, r(rScratch)[TVOFF(m_type)]);
       } else {
         emitImmReg(astubs, uintptr_t(&init_null_variant), r(rScratch));
       }

hphp/runtime/vm/translator/translator-x64.cpp

@@ -52,6 +52,7 @@ typedef __sighandler_t *sighandler_t;
 #include "util/ringbuffer.h"
 #include "util/timer.h"
 #include "util/trace.h"
+#include "util/meta.h"
 #include "util/util.h"
 
 #include "runtime/vm/bytecode.h"
@@ -123,6 +124,9 @@ enum TransPerfCounter {
 static __thread int64_t s_perfCounters[tpc_num_counters];
 #define INC_TPC(n) ++s_perfCounters[tpc_ ## n];
 
+#define KindOfString \
+#error You probably do not mean to use KindOfString in this file.
+
 #define NULLCASE() \
   case KindOfUninit: case KindOfNull
 
@@ -277,7 +281,8 @@ typeCanBeStatic(DataType t) {
 struct IfCountNotStatic {
   typedef CondBlock<FAST_REFCOUNT_OFFSET,
                     RefCountStaticValue,
-                    CC_Z> NonStaticCondBlock;
+                    CC_Z,
+                    field_type(RefData, _count)> NonStaticCondBlock;
   NonStaticCondBlock *m_cb; // might be null
   IfCountNotStatic(X64Assembler& a,
                    PhysReg reg,
@@ -418,7 +423,7 @@ void TranslatorX64::emitTvSetRegSafe(const NormalizedInstruction& i,
     emitDerefIfVariant(a, toPtr);
   }
 
-  a.  loadl  (toPtr[toOffset + TVOFF(m_type)], r32(oldType));
+  emitLoadTVType(a, toPtr[toOffset + TVOFF(m_type)], r32(oldType));
   a.  loadq  (toPtr[toOffset + TVOFF(m_data)], oldData);
   emitStoreTypedValue(a, fromType, from, toOffset, toPtr);
   if (incRefFrom) {
@@ -1212,7 +1217,7 @@ void TranslatorX64::emitGenericDecRefHelpers() {
   emitMovRegReg(a, rVmSp, rdi);
   // fall through
   m_dtorGenericStub = a.code.frontier;
-  a.    loadl  (rdi[TVOFF(m_type)], r32(rScratch));
+  emitLoadTVType(a, rdi[TVOFF(m_type)], r32(rScratch));
   a.    loadq  (rdi[TVOFF(m_data)], rdi);
   // Fall through to the regs stub.
 
@@ -2299,10 +2304,8 @@ TranslatorX64::emitPrologue(Func* func, int nPassed) {
 
       // rVmFp + rcx points to the count/type fields of the TypedValue we're
       // about to write to.
-      int loopStart = -func->numLocals() * sizeof(TypedValue)
-        + TVOFF(m_type);
-      int loopEnd = -numParams * sizeof(TypedValue)
-        + TVOFF(m_type);
+      int loopStart = -func->numLocals() * sizeof(TypedValue) + TVOFF(m_type);
+      int loopEnd = -numParams * sizeof(TypedValue) + TVOFF(m_type);
 
       emitImmReg(a, loopStart, loopReg);
       emitImmReg(a, KindOfUninit, rdx);
@@ -2324,7 +2327,7 @@ TranslatorX64::emitPrologue(Func* func, int nPassed) {
       }
       for (k = numParams; k < func->numLocals(); ++k) {
         locToRegDisp(Location(Location::Local, k), &base, &disp);
-        a.storel (eax, base[disp + TVOFF(m_type)]);
+        emitStoreTVType(a, eax, base[disp + TVOFF(m_type)]);
       }
     }
   }
@@ -2730,7 +2733,7 @@ TranslatorX64::emitStringCheck(X64Assembler& _a,
   // Treat KindOfString and KindOfStaticString identically; they
   // are bitwise identical. This is a port of our IS_STRING_TYPE
   // macro to assembly, and will have to change in sync with it.
-  _a.test_imm32_disp_reg32(KindOfStringBit, offset, base);
+  emitTestTVType(_a, KindOfStringBit, r64(base)[offset]);
 }
 
 void
@@ -2738,7 +2741,7 @@ TranslatorX64::emitCheckUncounted(X64Assembler& a,
                                   PhysReg       baseReg,
                                   int           offset,
                                   SrcRec&       fail) {
-  a.cmp_imm32_disp_reg32(KindOfStaticString, offset, baseReg);
+  emitCmpTVType(a, KindOfStaticString, r64(baseReg)[offset]);
   emitFallbackJmp(a, fail, CC_G);
 }
 
@@ -2747,7 +2750,7 @@ TranslatorX64::emitCheckUncountedInit(X64Assembler& a,
                                       PhysReg       baseReg,
                                       int           offset,
                                       SrcRec&       fail) {
-  a.test_imm32_disp_reg32(KindOfUncountedInitBit, offset, baseReg);
+  emitTestTVType(a, KindOfUncountedInitBit, r64(baseReg)[offset]);
   emitFallbackJmp(a, fail, CC_Z);
 }
 
@@ -2777,7 +2780,7 @@ TranslatorX64::emitTypeCheck(X64Assembler& _a, DataType dt,
       break;
     default:
       assert(IS_REAL_TYPE(dt));
-      _a. cmp_imm32_disp_reg32(dt, offset, base);
+      emitCmpTVType(_a, dt, r64(base)[offset]);
       if (fail) {
         emitFallbackJmp(*fail);
       }
@@ -4809,8 +4812,8 @@ TranslatorX64::translateCGetL(const Tracelet& t,
       ScratchReg rTmp(m_regMap);
       PhysReg localReg = getReg(inputs[0]->location);
       a.store_reg64_disp_reg64(localReg, stackDisp + TVOFF(m_data), stackBase);
-      a.load_reg64_disp_reg32(locBase, locDisp + TVOFF(m_type), r(rTmp));
-      a.store_reg32_disp_reg64(r(rTmp), stackDisp + TVOFF(m_type), stackBase);
+      emitLoadTVType(a, locBase[locDisp + TVOFF(m_type)], r(rTmp));
+      emitStoreTVType(a, r(rTmp), stackBase[stackDisp + TVOFF(m_type)]);
     }
     return;
   }
@@ -5048,8 +5051,8 @@ TranslatorX64::translateAssignToLocalOp(const Tracelet& t,
       ScratchReg rTmp(m_regMap);
       locToRegDisp(ni.inputs[rhsIdx]->location, &base, &disp);
       a.store_reg64_disp_reg64(rhsReg, TVOFF(m_data), localReg);
-      a.load_reg64_disp_reg32(base, disp + TVOFF(m_type), r(rTmp));
-      a.store_reg32_disp_reg64(r(rTmp), TVOFF(m_type), localReg);
+      emitLoadTVType(a, base[disp + TVOFF(m_type)], r(rTmp));
+      emitStoreTVType(a, r(rTmp), localReg[TVOFF(m_type)]);
     } else {
       emitStoreTypedValue(a, rhsType, rhsReg, 0, localReg);
     }
@@ -5063,8 +5066,8 @@ TranslatorX64::translateAssignToLocalOp(const Tracelet& t,
     locToRegDisp(ni.inputs[locIdx]->location, &locBase, &locDisp);
     ScratchReg rTmp(m_regMap);
     a.store_reg64_disp_reg64(rhsReg, locDisp + TVOFF(m_data), locBase);
-    a.load_reg64_disp_reg32(rhsBase, rhsDisp + TVOFF(m_type), r(rTmp));
-    a.store_reg32_disp_reg64(r(rTmp), locDisp + TVOFF(m_type), locBase);
+    emitLoadTVType(a, rhsBase[rhsDisp + TVOFF(m_type)], r(rTmp));
+    emitStoreTVType(a, r(rTmp), locBase[locDisp + TVOFF(m_type)]);
     m_regMap.swapRegisters(rhsReg, localReg);
     decRefType = oldLocalType;
     m_regMap.markAsClean(ni.inputs[locIdx]->location);
@@ -5686,7 +5689,7 @@ TranslatorX64::translateCns(const Tracelet& t,
   // Load the constant out of the thread-private tl_targetCaches.
   ScratchReg cns(m_regMap);
   a.    lea_reg64_disp_reg64(rVmTl, ch, r(cns));
-  a.    cmp_imm32_disp_reg32(0, TVOFF(m_type), r(cns));
+  emitCmpTVType(a, 0, r(cns)[TVOFF(m_type)]);
   DiamondReturn astubsRet;
   int stackDest = 0 - int(sizeof(Cell)); // popped - pushed
   {
@@ -5793,7 +5796,7 @@ TranslatorX64::translateClsCnsD(const Tracelet& t,
   CacheHandle ch = allocClassConstant(fullName);
   ScratchReg cns(m_regMap);
   a.lea_reg64_disp_reg64(rVmTl, ch, r(cns));
-  a.cmp_imm32_disp_reg32(0, TVOFF(m_type), r(cns));
+  emitCmpTVType(a, 0, r(cns)[TVOFF(m_type)]);
   {
     UnlikelyIfBlock ifNull(CC_Z, a, astubs);
 
@@ -6594,8 +6597,7 @@ void TranslatorX64::emitReturnVal(
         a.  load_reg64_disp_reg64(thisBase, thisOffset, scratch);
         a.  store_reg64_disp_reg64(scratch, dstOffset, dstBase);
       }
-      emitStoreImm(a, KindOfObject,
-                   dstBase, dstOffset + TVOFF(m_type), sz::dword);
+      emitStoreTVType(a, KindOfObject, dstBase[dstOffset + TVOFF(m_type)]);
       return;
     }
     case OpBareThis: {
@@ -6606,7 +6608,7 @@ void TranslatorX64::emitReturnVal(
         JccBlock<CC_NZ> noThis(a);
         a.  mov_imm32_reg32(KindOfObject, scratch);
       }
-      a.   store_reg32_disp_reg64(scratch, dstOffset + TVOFF(m_type), dstBase);
+      emitStoreTVType(a, scratch, dstBase[dstOffset + TVOFF(m_type)]);
       if (thisBase != dstBase || thisOffset != dstOffset) {
         a.  load_reg64_disp_reg64(thisBase, thisOffset, scratch);
         a.  store_reg64_disp_reg64(scratch, dstOffset, dstBase);
@@ -6617,8 +6619,7 @@ void TranslatorX64::emitReturnVal(
       not_reached();
   }
 
-  emitStoreImm(a, tv.m_type,
-               dstBase, dstOffset + TVOFF(m_type), sz::dword);
+  emitStoreTVType(a, tv.m_type, r64(dstBase)[dstOffset + TVOFF(m_type)]);
   if (tv.m_type != KindOfNull) {
     emitStoreImm(a, tv.m_data.num,
                  dstBase, dstOffset, sz::qword);
@@ -6747,7 +6748,7 @@ void TranslatorX64::emitInlineReturn(Location retvalSrcLoc,
       PhysReg base;
       int disp;
       locToRegDisp(m_curNI->inputs[k]->location, &base, &disp);
-      a.    storel (0, base[disp + TVOFF(m_type)]);
+      emitStoreTVType(a, KindOfUninit, base[disp + TVOFF(m_type)]);
     }
   }
 
@@ -7345,7 +7346,7 @@ void TranslatorX64::translateCreateCont(const Tracelet& t,
         // know it's an Object
         a.add_imm32_disp_reg32(1, FAST_REFCOUNT_OFFSET, r(rScratch));
         a.store_reg64_disp_reg64(r(rScratch), thisOff + TVOFF(m_data), r(rDest));
-        a.store_imm32_disp_reg(KindOfObject, thisOff + TVOFF(m_type), r(rDest));
+        emitStoreTVType(a, KindOfObject, r(rDest)[thisOff + TVOFF(m_type)]);
       }
     }
   } else {
@@ -9384,7 +9385,7 @@ TranslatorX64::translateBareThis(const Tracelet &t,
   DiamondReturn astubsRet;
   {
     UnlikelyIfBlock ifThisNull(CC_NZ, a, astubs, &astubsRet);
-    astubs. store_imm32_disp_reg(KindOfNull, TVOFF(m_type) + offset, base);
+    emitStoreTVType(astubs, KindOfNull, base[offset + TVOFF(m_type)]);
     if (i.imm[0].u_OA) {
       EMIT_CALL(astubs, warnNullThis);
       recordReentrantStubCall(i);
@@ -9403,7 +9404,7 @@ TranslatorX64::translateBareThis(const Tracelet &t,
   }
   emitIncRef(out, KindOfObject);
   if (i.outStack->rtt.isVagueValue()) {
-    a. store_imm32_disp_reg(KindOfObject, TVOFF(m_type) + offset, base);
+    emitStoreTVType(a, KindOfObject, base[offset + TVOFF(m_type)]);
     a. store_reg64_disp_reg64(out, TVOFF(m_data) + offset, base);
   } else {
     assert(i.outStack->isObject());
@@ -9445,7 +9446,7 @@ TranslatorX64::translateInitThisLoc(const Tracelet& t,
   a.jnz(astubs.code.frontier); // jnz if_null
 
   // We have a valid $this!
-  a.store_imm32_disp_reg(KindOfObject, offset + TVOFF(m_type), base);
+  emitStoreTVType(a, KindOfObject, base[offset + TVOFF(m_type)]);
   a.store_reg64_disp_reg64(r(thiz), offset + TVOFF(m_data), base);
   emitIncRef(r(thiz), KindOfObject);
 
@@ -9780,11 +9781,10 @@ TranslatorX64::translateFPushCufOp(const Tracelet& t,
       a.   load_reg64_disp_reg64(base1, TVOFF(m_data) + disp1, r(tmp));
       a.   store_reg64_disp_reg64(r(tmp), TVOFF(m_data) + disp2, base2);
       if (!inDef->rtt.isVagueValue()) {
-        a. store_imm32_disp_reg(inDef->outerType(),
-                                TVOFF(m_type) + disp2, base2);
+        emitStoreTVType(a, inDef->outerType(), base2[disp2 + TVOFF(m_type)]);
       } else {
-        a. load_reg64_disp_reg32(base1, TVOFF(m_type) + disp1, r(tmp));
-        a. store_reg32_disp_reg64(r(tmp), TVOFF(m_type) + disp2, base2);
+        emitLoadTVType(a,  base1[TVOFF(m_type) + disp1], r(tmp));
+        emitStoreTVType(a, r(tmp), base2[disp2 + TVOFF(m_type)]);
       }
     } else {
       PhysReg reg = m_regMap.getReg(inDef->location);
@@ -10065,19 +10065,19 @@ void TranslatorX64::translateFCallBuiltin(const Tracelet& t,
         emitStoreTypedValue(a, func->returnType(), rax, disp, base, true);
 
         ifNotZero.Else();
-        a.   storel  (KindOfNull, base[disp + TVOFF(m_type)]);
+        emitStoreTVType(a, KindOfNull, base[disp + TVOFF(m_type)]);
       }
       break;
     case KindOfUnknown:
       emitLea(a, returnBase, returnOffset, rax);
-      a.   loadq  (rax[TVOFF(m_type)], rScratch);
+      emitLoadTVType(a, rax[TVOFF(m_type)], rScratch);
       a.   cmpl   (KindOfUninit, r32(rScratch));
       {
         IfElseBlock<CC_Z> ifNotUninit(a);
         emitCopyToAligned(a, rax, 0, base, disp);
 
         ifNotUninit.Else();
-        a.   storel  (KindOfNull, base[disp + TVOFF(m_type)]);
+        emitStoreTVType(a, KindOfNull, base[disp + TVOFF(m_type)]);
       }
       break;
     default:
@@ -11545,7 +11545,7 @@ asm_label(a, release);
   });
   a.    ret    ();
 asm_label(a, doRelease);
-  a.    storel (0, rIter[TVOFF(m_type)]);
+  emitStoreTVType(a, KindOfUninit, rIter[TVOFF(m_type)]);
   jumpDestructor(a, PhysReg(rType), rax);
 
   m_freeManyLocalsHelper = a.code.frontier;
@@ -11554,7 +11554,7 @@ asm_label(a, doRelease);
   auto emitDecLocal = [&] {
     Label skipDecRef;
 
-    a.  loadl  (rIter[TVOFF(m_type)], rType);
+    emitLoadTVType(a, rIter[TVOFF(m_type)], rType);
     a.  cmpl   (KindOfRefCountThreshold, rType);
     a.  jle8   (skipDecRef);
     a.  call   (release);

hphp/runtime/vm/translator/translator-x64.h

@@ -116,7 +116,7 @@ class TranslatorX64 : public Translator
   friend class HPHP::VM::JIT::CodeGenerator;
   friend class HPHP::VM::JIT::HhbcTranslator; // packBitVec()
   friend TCA funcBodyHelper(ActRec* fp);
-  template<int, int, ConditionCode> friend class CondBlock;
+  template<int, int, ConditionCode, class> friend class CondBlock;
   template<ConditionCode, typename smasher> friend class JccBlock;
   template<ConditionCode> friend class IfElseBlock;
   friend class UnlikelyIfBlock;

hphp/util/asm-x64.h

@@ -137,6 +137,7 @@ struct RegRIP {
 inline Reg8 rbyte(Reg32 r)     { return Reg8(int(r)); }
 inline Reg8 rbyte(RegNumber r) { return Reg8(int(r)); }
 inline Reg32 r32(Reg8 r)       { return Reg32(int(r)); }
+inline Reg32 r32(Reg32 r)      { return r; }
 inline Reg32 r32(RegNumber r)  { return Reg32(int(r)); }
 inline Reg64 r64(RegNumber r)  { return Reg64(int(r)); }
 
@@ -1619,8 +1620,11 @@ struct X64Assembler {
   //     ir cannot be set to 'sp'
   void emitCMX(X64Instr op, int jcond, RegNumber brName,
                RegNumber irName, int s, int disp,
-               RegNumber rName, bool reverse, ssize_t imm,
-               bool hasImmediate, int opSz = sz::qword,
+               RegNumber rName,
+               bool reverse = false,
+               ssize_t imm = 0,
+               bool hasImmediate = false,
+               int opSz = sz::qword,
                bool ripRelative = false) ALWAYS_INLINE {
     assert(irName != reg::rsp);
 
@@ -1960,8 +1964,17 @@ public:
   // CMOVcc [rbase + off], rdest
   inline void cload_reg64_disp_reg64(ConditionCode cc, RegNumber rbase,
                                      int off, RegNumber rdest) {
-    emitCMX(instr_cmovcc, cc, rbase, reg::noreg, sz::byte, off,
-            rdest, false, 0, false);
+    emitCMX(instr_cmovcc, cc, rbase, reg::noreg, sz::byte, off, rdest,
+            false /*reverse*/);
+
+  }
+  inline void cload_reg64_disp_reg32(ConditionCode cc, RegNumber rbase,
+                                     int off, RegNumber rdest) {
+    emitCMX(instr_cmovcc, cc, rbase, reg::noreg, sz::byte, off, rdest,
+            false /*reverse*/,
+            0 /*imm*/,
+            false /*hasImmediate*/,
+            sz::dword /*opSz*/);
   }
   inline void cmov_reg64_reg64(ConditionCode cc, RegNumber rsrc,
                                RegNumber rdest) {

hphp/util/meta.h

@@ -40,7 +40,8 @@ struct match_iterator : boost::mpl::eval_if<
   boost::add_const<Value>,
   boost::remove_const<Value> > {};
 
-
+// field_type: provide the type of a struct field.
+#define field_type(strct, fld) decltype(((strct*)0)->fld)
 
 }
 

hphp/util/test/asm.cpp

@@ -555,6 +555,19 @@ test %sil,(%rcx,%rsi,8)
 )");
 }
 
+TEST(Asm, CMov) {
+  Asm a;
+  a.init(10 << 24);
+  a.   test_reg64_reg64(rax, rax);
+  a.   cload_reg64_disp_reg64(CC_Z, rax, 0, rax);
+  a.   cload_reg64_disp_reg32(CC_Z, rax, 0, rax);
+  expect_asm(a, R"(
+test %rax,%rax
+cmove (%rax),%rax
+cmove (%rax),%eax
+)");
+}
+
 TEST(Asm, SimpleLabelTest) {
   Asm a;
   a.init(10 << 24);