Adding shift left and right to codegen

shift left and right in codegen

hphp/doc/ir.specification

@@ -469,6 +469,8 @@ D:{Int|Bool|Dbl} = OpDiv    S0:{Int|Dbl} S1:{Int|Dbl}
 D:Int            = OpBitAnd S0:Int S1:Int
 D:Int            = OpBitOr  S0:Int S1:Int
 D:Int            = OpBitXor S0:Int S1:Int
+D:Int            = OpShl    S0:Int S1:Int
+D:Int            = OpShr    S0:Int S1:Int
 
   Integer/boolean arithmetic.  Performs the operation described by the
   opcode name on S0 and S1, and puts the result in D.
@@ -480,6 +482,8 @@ D:Int            = OpBitXor S0:Int S1:Int
   not closed under OpDiv, as OpDiv produces doubles when there is a
   non-zero remainder.
 
+  Note that OpShr is an arithmetic right shift.
+
 D:Bool = OpLogicXor S0:Bool S1:Bool
 
   Logical XOR of the two sources. (Note that && and || do not have

hphp/runtime/vm/jit/code-gen.cpp

@@ -1521,6 +1521,98 @@ void CodeGenerator::cgOpMod(IRInstruction* inst) {
   }
 }
 
+template<class Oper>
+void CodeGenerator::cgOpShiftCommon(IRInstruction* inst,
+                                    void (Asm::*instrIR)(Immed, Reg64),
+                                    void (Asm::*instrR)(Reg64),
+                                    Oper oper) {
+  const SSATmp* dst   = inst->dst();
+  const SSATmp* src1  = inst->src(0);
+  const SSATmp* src2  = inst->src(1);
+
+  auto const srcReg1 = m_regs[src1].reg();
+  auto const srcReg2 = m_regs[src2].reg();
+  auto const dstReg  = m_regs[dst].reg();
+
+  // two immediates
+  if (srcReg1 == InvalidReg && srcReg2 == InvalidReg) {
+    assert(src1->isConst() && src2->isConst());
+    int64_t value = oper(src1->getValInt(), src2->getValInt());
+    emitLoadImm(m_as, value, dstReg);
+    return;
+  }
+
+  // one immediate (right), see below for a lhs immediate
+  if (srcReg2 == InvalidReg) {
+    assert(src2->isConst() && src2->type() == Type::Int);
+    emitMovRegReg(m_as, srcReg1, dstReg);
+    (m_as.*instrIR)(src2->getValInt(), dstReg);
+    return;
+  }
+
+  // in order to shift by a variable amount src2 must be in rcx :(
+  bool swapRCX = srcReg2 != reg::rcx;
+
+  // will we be using dstReg as scratch storage?
+  bool dstIsRHS = dstReg == srcReg2;
+  bool tmpIsRCX = m_rScratch == reg::rcx;
+  bool dstIsRCX = dstReg == reg::rcx;
+
+  // we need rcx for srcReg2 so we use srcReg2 as a temp for rcx, we also need
+  // to handle the cases where the destination is rcx or src2 or both...
+  auto resReg = dstIsRCX ? (dstIsRHS ? PhysReg(m_rScratch) : srcReg2)
+                         : (dstIsRHS ? (tmpIsRCX ? dstReg : PhysReg(m_rScratch))
+                                     : dstReg);
+
+  // if srcReg1 was in rcx it will be swapped with srcReg2 below
+  auto regLeft = srcReg1 == reg::rcx ? srcReg2 : srcReg1;
+
+  // we use srcReg2 as a scratch for whatever is in rcx
+  if (swapRCX) {
+    m_as.   xchgq(reg::rcx, srcReg2);
+  }
+
+  // one immeidate (left)
+  if (srcReg1 == InvalidReg) {
+    assert(src1->isConst());
+    emitLoadImm(m_as, src1->getValInt(), resReg);
+  } else {
+    emitMovRegReg(m_as, regLeft, resReg);
+  }
+
+  (m_as.*instrR)(resReg);
+
+  if (resReg == dstReg && srcReg2 == dstReg) {
+    // If we get here it means that m_rScratch was rcx and we shouldn't do any
+    // more swapping because we stored the result in the right place
+    return;
+  }
+
+  if (swapRCX) {
+    m_as.   xchgq(reg::rcx, srcReg2);
+  }
+
+  // if resReg == srcReg2 then dstReg must have been rcx and the above swap
+  // already repaired the situation
+  if (resReg != srcReg2) {
+    emitMovRegReg(m_as, resReg, dstReg);
+  }
+}
+
+void CodeGenerator::cgOpShl(IRInstruction* inst) {
+  cgOpShiftCommon(inst,
+                  &Asm::shlq,
+                  &Asm::shlq,
+                  [] (int64_t a, int64_t b) { return a << b; });
+}
+
+void CodeGenerator::cgOpShr(IRInstruction* inst) {
+  cgOpShiftCommon(inst,
+                  &Asm::sarq,
+                  &Asm::sarq,
+                  [] (int64_t a, int64_t b) { return a >> b; });
+}
+
 void CodeGenerator::cgOpNot(IRInstruction* inst) {
   auto const src = inst->src(0);
   auto const dstReg = m_regs[inst->dst()].reg();

hphp/runtime/vm/jit/code-gen.h

@@ -227,6 +227,12 @@ private:
                      RegType (*conv)(PhysReg),
                      Commutativity);
 
+  template<class Oper>
+  void cgOpShiftCommon(IRInstruction* inst,
+                       void (Asm::*instrIR)(Immed, Reg64),
+                       void (Asm::*instrR)(Reg64),
+                       Oper oper);
+
   void cgNegateWork(SSATmp* dst, SSATmp* src);
   void cgNotWork(SSATmp* dst, SSATmp* src);
 

hphp/runtime/vm/jit/hhbc-translator.cpp

@@ -3434,6 +3434,30 @@ void HhbcTranslator::emitXor() {
   gen(DecRef, btr);
 }
 
+void HhbcTranslator::emitShl() {
+  auto shiftAmount = popC();
+  auto lhs         = popC();
+
+  auto lhsInt         = gen(ConvCellToInt, lhs);
+  auto shiftAmountInt = gen(ConvCellToInt, shiftAmount);
+
+  push(gen(OpShl, lhsInt, shiftAmountInt));
+  gen(DecRef, lhs);
+  gen(DecRef, shiftAmount);
+}
+
+void HhbcTranslator::emitShr() {
+  auto shiftAmount = popC();
+  auto lhs         = popC();
+
+  auto lhsInt         = gen(ConvCellToInt, lhs);
+  auto shiftAmountInt = gen(ConvCellToInt, shiftAmount);
+
+  push(gen(OpShr, lhsInt, shiftAmountInt));
+  gen(DecRef, lhs);
+  gen(DecRef, shiftAmount);
+}
+
 namespace {
 
 Type arithOpResult(Type t1, Type t2) {

hphp/runtime/vm/jit/hhbc-translator.h

@@ -338,6 +338,8 @@ struct HhbcTranslator {
   void emitMul();
   void emitMod();
   void emitDiv();
+  void emitShl();
+  void emitShr();
 
   // boolean ops
   void emitXor();

hphp/runtime/vm/jit/ir-translator.cpp

@@ -471,6 +471,16 @@ IRTranslator::translateBitNot(const NormalizedInstruction& i) {
   HHIR_EMIT(BitNot);
 }
 
+void
+IRTranslator::translateShl(const NormalizedInstruction& i) {
+  HHIR_EMIT(Shl);
+}
+
+void
+IRTranslator::translateShr(const NormalizedInstruction& i) {
+  HHIR_EMIT(Shr);
+}
+
 void
 IRTranslator::translateCastInt(const NormalizedInstruction& i) {
   assert(i.inputs.size() == 1);

hphp/runtime/vm/jit/ir.h

@@ -219,6 +219,8 @@ O(OpBitXor,                     D(Int), S(Int) S(Int),                     C) \
 O(OpBitNot,                     D(Int), S(Int),                            C) \
 O(OpLogicXor,                  D(Bool), S(Bool) S(Bool),                   C) \
 O(OpNot,                       D(Bool), S(Bool),                           C) \
+O(OpShl,                        D(Int), S(Int) S(Int),                     C) \
+O(OpShr,                        D(Int), S(Int) S(Int),                     C) \
                                                                               \
 O(ConvBoolToArr,                D(Arr), S(Bool),                         C|N) \
 O(ConvDblToArr,                 D(Arr), S(Dbl),                          C|N) \

hphp/runtime/vm/jit/simplifier.cpp

@@ -303,6 +303,8 @@ SSATmp* Simplifier::simplify(IRInstruction* inst) {
   case OpBitOr:     return simplifyBitOr(src1, src2);
   case OpBitXor:    return simplifyBitXor(src1, src2);
   case OpLogicXor:  return simplifyLogicXor(src1, src2);
+  case OpShl:       return simplifyShl(inst);
+  case OpShr:       return simplifyShr(inst);
 
   case OpGt:
   case OpGte:
@@ -1002,6 +1004,41 @@ SSATmp* Simplifier::simplifyLogicXor(SSATmp* src1, SSATmp* src2) {
   return nullptr;
 }
 
+template<class Oper>
+SSATmp* Simplifier::simplifyShift(SSATmp* src1, SSATmp* src2, Oper op) {
+  if (src1->isConst()) {
+    if (src1->getValInt() == 0) {
+      return cns(0);
+    }
+
+    if (src2->isConst()) {
+      return cns(op(src1->getValInt(), src2->getValInt()));
+    }
+  }
+
+  if (src2->isConst() && src2->getValInt() == 0) {
+    return src1;
+  }
+
+  return nullptr;
+}
+
+SSATmp* Simplifier::simplifyShl(IRInstruction* inst) {
+  auto src1 = inst->src(0);
+  auto src2 = inst->src(1);
+
+  return simplifyShift(src1, src2, [] (int64_t a, int64_t b) {
+                       return a << b; });
+}
+
+SSATmp* Simplifier::simplifyShr(IRInstruction* inst) {
+  auto src1 = inst->src(0);
+  auto src2 = inst->src(1);
+
+  return simplifyShift(src1, src2, [] (int64_t a, int64_t b) {
+                       return a >> b; });
+}
+
 static SSATmp* chaseIncRefs(SSATmp* tmp) {
   while (tmp->inst()->op() == IncRef) {
     tmp = tmp->inst()->src(0);

hphp/runtime/vm/jit/simplifier.h

@@ -71,6 +71,8 @@ private:
   SSATmp* simplifyBitAnd(SSATmp* src1, SSATmp* src2);
   SSATmp* simplifyBitOr(SSATmp* src1, SSATmp* src2);
   SSATmp* simplifyBitXor(SSATmp* src1, SSATmp* src2);
+  SSATmp* simplifyShl(IRInstruction* inst);
+  SSATmp* simplifyShr(IRInstruction* inst);
   SSATmp* simplifyLogicXor(SSATmp* src1, SSATmp* src2);
   SSATmp* simplifyGt(SSATmp* src1, SSATmp* src2);
   SSATmp* simplifyGte(SSATmp* src1, SSATmp* src2);
@@ -136,6 +138,9 @@ private:
   SSATmp* simplifyStRef(IRInstruction*);
   SSATmp* simplifyAssertNonNull(IRInstruction*);
 
+  template<class Oper>
+  SSATmp* simplifyShift(SSATmp* src1, SSATmp* src2, Oper op);
+
 private: // tracebuilder forwarders
   template<class... Args> SSATmp* cns(Args&&...);
   template<class... Args> SSATmp* gen(Opcode op, Args&&...);

hphp/runtime/vm/jit/translator-instrs.h

@@ -165,6 +165,8 @@
   CASE(FPassV) \
   CASE(UnsetN) \
   CASE(DecodeCufIter) \
+  CASE(Shl) \
+  CASE(Shr)
 
   // These are instruction-like functions which cover more than one
   // opcode.

hphp/util/asm-x64.h

@@ -1175,11 +1175,15 @@ public:
 
   void shlq  (Immed i, Reg64 r) { instrIR(instr_shl, i.b(), r); }
   void shrq  (Immed i, Reg64 r) { instrIR(instr_shr, i.b(), r); }
+  void sarq  (Immed i, Reg64 r) { instrIR(instr_sar, i.b(), r); }
   void shll  (Immed i, Reg32 r) { instrIR(instr_shl, i.b(), r); }
   void shrl  (Immed i, Reg32 r) { instrIR(instr_shr, i.b(), r); }
   void shlw  (Immed i, Reg16 r) { instrIR(instr_shl, i.b(), r); }
   void shrw  (Immed i, Reg16 r) { instrIR(instr_shr, i.b(), r); }
 
+  void shlq (Reg64 r) { instrR(instr_shl, r); }
+  void sarq (Reg64 r) { instrR(instr_sar, r); }
+
   /*
    * Control-flow directives.  Primitive labeling/patching facilities
    * are available, as well as slightly higher-level ones via the

hphp/util/test/asm.cpp

@@ -704,4 +704,24 @@ asm_label(a, loop);
   test_case(127);
 }
 
+TEST(Asm, ShiftingWithCl) {
+  Asm a;
+  a.init(10 << 24);
+
+  a.    shlq(rax);
+  a.    shlq(rdx);
+  a.    shlq(r8);
+  a.    sarq(rbx);
+  a.    sarq(rsi);
+  a.    sarq(r8);
+  expect_asm(a, R"(
+shl %cl,%rax
+shl %cl,%rdx
+shl %cl,%r8
+sar %cl,%rbx
+sar %cl,%rsi
+sar %cl,%r8
+)");
+}
+
 }}