From daae3730e1fb882e93e29389f6863df88cd59ba0 Mon Sep 17 00:00:00 2001 From: Owen Yamauchi Date: Wed, 15 May 2013 13:10:10 -0700 Subject: [PATCH] Split Xor into BitXor and LogicXor; clean up binary arithmetic This started out as splitting Xor into two separate opcodes with more precise type descriptions, and turned into a more sweeping change that removes the conditional "use bool ops" logic in binary arithmetic, by simply emitting ConvBoolToInt for bool inputs. This will actually emit more code than before in the case of binary operators with two bool operands. As I write this I'm crossing my fingers and hoping that this will turn out not to matter in practice. Also deleted some redundant if cases in codegen. --- hphp/doc/ir.specification | 26 +- hphp/runtime/vm/translator/hopt/codegen.cpp | 270 +++++------------- hphp/runtime/vm/translator/hopt/codegen.h | 2 - .../vm/translator/hopt/hhbctranslator.cpp | 36 +-- hphp/runtime/vm/translator/hopt/ir.h | 25 +- .../vm/translator/hopt/irtranslator.cpp | 6 +- .../runtime/vm/translator/hopt/simplifier.cpp | 53 ++-- hphp/runtime/vm/translator/hopt/simplifier.h | 7 +- hphp/runtime/vm/translator/hopt/type.cpp | 12 +- 9 files changed, 176 insertions(+), 261 deletions(-) diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 5f862994a..956aeea5e 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -380,14 +380,14 @@ GuardRefs 2. Arithmetic -D:Int = OpAdd S0:{Int|Bool} S1:{Int|Bool} -D:Int = OpSub S0:{Int|Bool} S1:{Int|Bool} -D:Int = OpAnd S0:{Int|Bool} S1:{Int|Bool} -D:Int = OpOr S0:{Int|Bool} S1:{Int|Bool} -D:Int = OpXor S0:{Int|Bool} S1:{Int|Bool} -D:Int = OpMul S0:{Int|Bool} S1:{Int|Bool} -D:{Int|Bool} = OpMod S0:{Int|Bool} S1:{Int|Bool} -D:{Int|Bool|Dbl} = OpDiv S0:{Int|Bool} S1:{Int|Bool} +D:{Int|Dbl} = OpAdd S0:{Int|Dbl} S1:{Int|Dbl} +D:{Int|Dbl} = OpSub S0:{Int|Dbl} S1:{Int|Dbl} +D:{Int|Dbl} = OpMul S0:{Int|Dbl} S1:{Int|Dbl} +D:{Int|Bool} = OpMod S0:{Int|Dbl} S1:{Int|Dbl} +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 Integer/boolean arithmetic. Performs the operation described by the opcode name on S0 and S1, and puts the result in D. @@ -396,6 +396,16 @@ D:{Int|Bool|Dbl} = OpDiv S0:{Int|Bool} S1:{Int|Bool} zero-like. Ints are not closed under OpDiv, as OpDiv produces doubles when there is a non-zero remainder. +D:Bool = OpLogicXor S0:Bool S1:Bool + + Logical XOR of the two sources. (Note that && and || do not have + corresponding opcodes because they're handled at the bytecode level, + to implement short-circuiting.) + +D:Bool = OpNot S0:Bool + + Logical negation of the source. + 3. Type conversions diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 9f3a856c2..d02628b8a 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -516,14 +516,6 @@ void shuffle2(CodeGenerator::Asm& a, } } -static void signExtendBool(X64Assembler& as, const RegisterInfo& info) { - auto reg = info.getReg(); - if (reg != InvalidReg) { - // sign-extend the bool from a byte to a quad - as.movsbq(rbyte(reg), r64(reg)); - } -} - static void zeroExtendBool(X64Assembler& as, const RegisterInfo& info) { auto reg = info.getReg(); if (reg != InvalidReg) { @@ -967,18 +959,10 @@ void CodeGenerator::cgUnaryIntOp(SSATmp* dst, } } -void CodeGenerator::cgNotWork(SSATmp* dst, SSATmp* src) { - cgUnaryIntOp(dst, src, &Asm::not, [](int64_t i) { return ~i; }); -} - void CodeGenerator::cgNegateWork(SSATmp* dst, SSATmp* src) { cgUnaryIntOp(dst, src, &Asm::neg, [](int64_t i) { return -i; }); } -void CodeGenerator::cgNegate(IRInstruction* inst) { - cgNegateWork(inst->getDst(), inst->getSrc(0)); -} - inline static Reg8 convertToReg8(PhysReg reg) { return rbyte(reg); } inline static Reg64 convertToReg64(PhysReg reg) { return reg; } @@ -987,7 +971,6 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, void (Asm::*instrIR)(Immed, RegType), void (Asm::*instrRR)(RegType, RegType), void (Asm::*movInstr)(RegType, RegType), - void (*extendInstr)(Asm&, const RegisterInfo&), Oper oper, RegType (*convertReg)(PhysReg), Commutativity commuteFlag) { @@ -1004,28 +987,12 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, auto const src1Reg = m_regs[src1].getReg(); auto const src2Reg = m_regs[src2].getReg(); auto& a = m_as; - bool const useBoolOps = src1->isA(Type::Bool) && src2->isA(Type::Bool); auto const dstOpReg = convertReg(dstReg); auto const src1OpReg = convertReg(src1Reg); auto const src2OpReg = convertReg(src2Reg); auto const rOpScratch = convertReg(rScratch); - // We can only use byte operations if both operands are boolean. If so, we'll - // sign extend *after* the operation is complete to save a movzbl. - if (!useBoolOps) { - // Extend booleans: integer operations require 64-bit - // representations. - zeroExtendIfBool(m_as, src1, m_regs[src1]); - zeroExtendIfBool(m_as, src2, m_regs[src2]); - } - - auto signExtendIfNecessary = [&] { - if (useBoolOps) { - extendInstr(m_as, m_regs[dst]); - } - }; - // Two registers. if (src1Reg != InvalidReg && src2Reg != InvalidReg) { if (dstReg == src1Reg) { @@ -1042,16 +1009,13 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, emitMovRegReg(a, src1Reg, dstReg); (a.*instrRR) (src2OpReg, dstOpReg); } - signExtendIfNecessary(); return; } // Two immediates. if (src1Reg == InvalidReg && src2Reg == InvalidReg) { assert(src1->isConst() && src2->isConst()); - int64_t value = useBoolOps ? - (int64_t)oper(src1->getValBool(), src2->getValBool()) : - oper(src1->getValRawInt(), src2->getValRawInt()); + int64_t value = oper(src1->getValRawInt(), src2->getValRawInt()); emitLoadImm(a, value, dstReg); return; } @@ -1059,8 +1023,7 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, // One register, and one immediate. if (commutative) { auto immedSrc = (src2Reg == InvalidReg ? src2 : src1); - auto immed = useBoolOps ? - (int64_t)immedSrc->getValBool() : immedSrc->getValRawInt(); + auto immed = immedSrc->getValRawInt(); auto srcReg = m_regs[(src2Reg == InvalidReg ? src1 : src2)].getReg(); if (srcReg == dstReg) { (a.*instrIR) (immed, dstOpReg); @@ -1068,31 +1031,25 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, emitLoadImm(a, immed, dstReg); (a.*instrRR) (convertReg(srcReg), dstOpReg); } - signExtendIfNecessary(); return; } // NonCommutative: if (src1Reg == InvalidReg) { if (dstReg == src2Reg) { - emitLoadImm(a, useBoolOps ? - (int64_t)src1->getValBool() : src1->getValRawInt(), rScratch); + emitLoadImm(a, src1->getValRawInt(), rScratch); (a.*instrRR) (src2OpReg, rOpScratch); (a.*movInstr)(rOpScratch, dstOpReg); } else { - emitLoadImm(a, useBoolOps ? - (int64_t)src1->getValBool() : src1->getValRawInt(), dstReg); + emitLoadImm(a, src1->getValRawInt(), dstReg); (a.*instrRR) (src2OpReg, dstOpReg); } - signExtendIfNecessary(); return; } assert(src2Reg == InvalidReg); emitMovRegReg(a, src1Reg, dstReg); - (a.*instrIR) ( - useBoolOps ? src2->getValBool() : src2->getValRawInt(), dstOpReg); - signExtendIfNecessary(); + (a.*instrIR) (src2->getValRawInt(), dstOpReg); } template @@ -1101,7 +1058,6 @@ void CodeGenerator::cgBinaryOp(IRInstruction* inst, void (Asm::*instrRR)(RegType, RegType), void (Asm::*movInstr)(RegType, RegType), void (Asm::*fpInstr)(RegXMM, RegXMM), - void (*extendInstr)(Asm&, const RegisterInfo&), Oper oper, RegType (*convertReg)(PhysReg), Commutativity commuteFlag) { @@ -1120,7 +1076,7 @@ void CodeGenerator::cgBinaryOp(IRInstruction* inst, m_as. mov_xmm_reg64(xmm0, m_regs[dst].getReg()); return; } - cgBinaryIntOp(inst, instrIR, instrRR, movInstr, extendInstr, + cgBinaryIntOp(inst, instrIR, instrRR, movInstr, oper, convertReg, commuteFlag); } @@ -1162,26 +1118,14 @@ void CodeGenerator::cgOpAdd(IRInstruction* inst) { // Special cases: x = y + 1 if (emitInc(dst, src1, src2) || emitInc(dst, src2, src1)) return; - if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { - cgBinaryIntOp(inst, - &Asm::addb, - &Asm::addb, - &Asm::movb, - &zeroExtendBool, - std::plus(), - &convertToReg8, - Commutative); - } else { - cgBinaryOp(inst, - &Asm::addq, - &Asm::addq, - &Asm::movq, - &Asm::addsd_xmm_xmm, - nullptr, // not used - std::plus(), - &convertToReg64, - Commutative); - } + cgBinaryOp(inst, + &Asm::addq, + &Asm::addq, + &Asm::movq, + &Asm::addsd_xmm_xmm, + std::plus(), + &convertToReg64, + Commutative); } void CodeGenerator::cgOpSub(IRInstruction* inst) { @@ -1192,141 +1136,81 @@ void CodeGenerator::cgOpSub(IRInstruction* inst) { if (emitDec(dst, src1, src2)) return; if (src1->isConst() && src1->isA(Type::Int) && src1->getValInt() == 0 && - !src2->isA(Type::Dbl)) { - return cgNegateWork(dst, src2); + src2->isA(Type::Int)) { + cgNegateWork(dst, src2); + return; } - if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { - cgBinaryIntOp(inst, - &Asm::subb, - &Asm::subb, - &Asm::movb, - &signExtendBool, // bool "-1" needs to be sign extended as int - std::minus(), - &convertToReg8, - NonCommutative); - } else { - cgBinaryOp(inst, - &Asm::subq, - &Asm::subq, - &Asm::movq, - &Asm::subsd_xmm_xmm, - nullptr, // not used - std::minus(), - &convertToReg64, - NonCommutative); - } -} - -void CodeGenerator::cgOpAnd(IRInstruction* inst) { - SSATmp* src1 = inst->getSrc(0); - SSATmp* src2 = inst->getSrc(1); - - if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { - cgBinaryIntOp(inst, - &Asm::andb, - &Asm::andb, - &Asm::movb, - &zeroExtendBool, - [] (bool a, bool b) { return a & b; }, - &convertToReg8, - Commutative); - } else { - cgBinaryIntOp(inst, - &Asm::andq, - &Asm::andq, - &Asm::movq, - nullptr, // not used - [] (int64_t a, int64_t b) { return a & b; }, - &convertToReg64, - Commutative); - } -} - -void CodeGenerator::cgOpOr(IRInstruction* inst) { - SSATmp* src1 = inst->getSrc(0); - SSATmp* src2 = inst->getSrc(1); - - if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { - cgBinaryIntOp(inst, - &Asm::orb, - &Asm::orb, - &Asm::movb, - &zeroExtendBool, - [] (bool a, bool b) { return a | b; }, - &convertToReg8, - Commutative); - } else { - cgBinaryIntOp(inst, - &Asm::orq, - &Asm::orq, - &Asm::movq, - nullptr, // not used - [] (int64_t a, int64_t b) { return a | b; }, - &convertToReg64, - Commutative); - } + cgBinaryOp(inst, + &Asm::subq, + &Asm::subq, + &Asm::movq, + &Asm::subsd_xmm_xmm, + std::minus(), + &convertToReg64, + NonCommutative); } void CodeGenerator::cgOpDiv(IRInstruction* inst) { not_implemented(); } -void CodeGenerator::cgOpXor(IRInstruction* inst) { - SSATmp* dst = inst->getDst(); - SSATmp* src1 = inst->getSrc(0); - SSATmp* src2 = inst->getSrc(1); - if (src2->isConst() && src2->type() == Type::Int && - src2->getValInt() == ~0L) { - return cgNotWork(dst, src1); - } +void CodeGenerator::cgOpBitAnd(IRInstruction* inst) { + cgBinaryIntOp(inst, + &Asm::andq, + &Asm::andq, + &Asm::movq, + [] (int64_t a, int64_t b) { return a & b; }, + &convertToReg64, + Commutative); +} - if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { - cgBinaryIntOp(inst, - &Asm::xorb, - &Asm::xorb, - &Asm::movb, - &zeroExtendBool, - [] (bool a, bool b) { return a ^ b; }, - &convertToReg8, - Commutative); - } else { - cgBinaryIntOp(inst, - &Asm::xorq, - &Asm::xorq, - &Asm::movq, - nullptr, // not used - [] (int64_t a, int64_t b) { return a ^ b; }, - &convertToReg64, - Commutative); - } +void CodeGenerator::cgOpBitOr(IRInstruction* inst) { + cgBinaryIntOp(inst, + &Asm::orq, + &Asm::orq, + &Asm::movq, + [] (int64_t a, int64_t b) { return a | b; }, + &convertToReg64, + Commutative); +} + +void CodeGenerator::cgOpBitXor(IRInstruction* inst) { + cgBinaryIntOp(inst, + &Asm::xorq, + &Asm::xorq, + &Asm::movq, + [] (int64_t a, int64_t b) { return a ^ b; }, + &convertToReg64, + Commutative); +} + +void CodeGenerator::cgOpBitNot(IRInstruction* inst) { + cgUnaryIntOp(inst->getDst(), + inst->getSrc(0), + &Asm::not, + [](int64_t i) { return ~i; }); +} + +void CodeGenerator::cgOpLogicXor(IRInstruction* inst) { + cgBinaryIntOp(inst, + &Asm::xorb, + &Asm::xorb, + &Asm::movb, + [] (bool a, bool b) { return a ^ b; }, + &convertToReg8, + Commutative); } void CodeGenerator::cgOpMul(IRInstruction* inst) { - SSATmp* src1 = inst->getSrc(0); - SSATmp* src2 = inst->getSrc(1); - - if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { - cgBinaryIntOp(inst, - &Asm::andb, - &Asm::andb, - &Asm::movb, - &zeroExtendBool, - [] (bool a, bool b) { return a & b; }, - &convertToReg8, - Commutative); - } else { - // Boolean multiplication is the same as & - cgBinaryOp(inst, - &Asm::imul, - &Asm::imul, - &Asm::movq, - &Asm::mulsd_xmm_xmm, - nullptr, // not used - std::multiplies(), - &convertToReg64, - Commutative); - } + cgBinaryOp(inst, + &Asm::imul, + &Asm::imul, + &Asm::movq, + &Asm::mulsd_xmm_xmm, + std::multiplies(), + &convertToReg64, + Commutative); } void CodeGenerator::cgOpNot(IRInstruction* inst) { diff --git a/hphp/runtime/vm/translator/hopt/codegen.h b/hphp/runtime/vm/translator/hopt/codegen.h index 55a57f999..78e3d34ad 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.h +++ b/hphp/runtime/vm/translator/hopt/codegen.h @@ -212,7 +212,6 @@ private: void (Asm::*intRR)(RegType, RegType), void (Asm::*mov)(RegType, RegType), void (Asm::*fpRR)(RegXMM, RegXMM), - void (*extend)(Asm&, const RegisterInfo&), Oper, RegType (*conv)(PhysReg), Commutativity); @@ -221,7 +220,6 @@ private: void (Asm::*intImm)(Immed, RegType), void (Asm::*intRR)(RegType, RegType), void (Asm::*mov)(RegType, RegType), - void (*extend)(Asm&, const RegisterInfo&), Oper, RegType (*conv)(PhysReg), Commutativity); diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index 2f8692814..d9d1eaac6 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -716,15 +716,20 @@ void HhbcTranslator::emitIncDecMem(bool pre, gen(StMemNT, propAddr, cns(0), res); } -static bool isSupportedBinaryArith(Opcode opc, Type t1, Type t2) { +static bool areBinaryArithTypesSupported(Opcode opc, Type t1, Type t2) { switch (opc) { case OpAdd: case OpSub: case OpMul: return t1.subtypeOfAny(Type::Int, Type::Bool, Type::Dbl) && t2.subtypeOfAny(Type::Int, Type::Bool, Type::Dbl); - default: return t1.subtypeOfAny(Type::Int, Type::Bool) && + case OpBitAnd: + case OpBitOr: + case OpBitXor: + return t1.subtypeOfAny(Type::Int, Type::Bool) && t2.subtypeOfAny(Type::Int, Type::Bool); + default: + not_reached(); } } @@ -745,9 +750,13 @@ void HhbcTranslator::emitSetOpL(Opcode subOpc, uint32_t id) { return; } - if (isSupportedBinaryArith(subOpc, loc->type(), topC()->type())) { + if (areBinaryArithTypesSupported(subOpc, loc->type(), topC()->type())) { auto const val = popC(); - auto const result = gen(subOpc, loc, val); + auto const result = gen( + subOpc, + loc->isA(Type::Bool) ? gen(ConvBoolToInt, loc) : loc, + val->isA(Type::Bool) ? gen(ConvBoolToInt, val) : val + ); push(stLoc(id, exitTrace, result)); return; } @@ -2804,16 +2813,15 @@ void HhbcTranslator::emitCGetS(const StringData* propName, } void HhbcTranslator::emitBinaryArith(Opcode opc) { - bool isBitOp = (opc == OpAnd || opc == OpOr || opc == OpXor); + bool isBitOp = (opc == OpBitAnd || opc == OpBitOr || opc == OpBitXor); Type type1 = topC(0)->type(); Type type2 = topC(1)->type(); - if (isSupportedBinaryArith(opc, type1, type2)) { + if (areBinaryArithTypesSupported(opc, type1, type2)) { SSATmp* tr = popC(); SSATmp* tl = popC(); + tr = (tr->isA(Type::Bool) ? gen(ConvBoolToInt, tr) : tr); + tl = (tl->isA(Type::Bool) ? gen(ConvBoolToInt, tl) : tl); push(gen(opc, tl, tr)); - } else if (isBitOp && (type1 == Type::Obj || type2 == Type::Obj)) { - // raise fatal - emitInterpOne(Type::Cell, 2); } else { Type type = Type::Int; if (isBitOp) { @@ -2899,13 +2907,9 @@ void HhbcTranslator::emitMod() { void HhbcTranslator::emitBitNot() { Type srcType = topC()->type(); - if (srcType == Type::Int) { + if (srcType.subtypeOf(Type::Int)) { SSATmp* src = popC(); - SSATmp* ones = cns(~uint64_t(0)); - push(gen(OpXor, src, ones)); - } else if (srcType.subtypeOf(Type::Null | Type::Bool | Type::Arr | Type::Obj)) { - // raise fatal - emitInterpOne(Type::Cell, 1); + push(gen(OpBitNot, src)); } else { Type resultType = Type::Int; if (srcType.isString()) { @@ -2922,7 +2926,7 @@ void HhbcTranslator::emitXor() { SSATmp* btl = popC(); SSATmp* tr = gen(ConvCellToBool, btr); SSATmp* tl = gen(ConvCellToBool, btl); - push(gen(ConvCellToBool, gen(OpXor, tl, tr))); + push(gen(ConvCellToBool, gen(OpLogicXor, tl, tr))); gen(DecRef, btl); gen(DecRef, btr); } diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 0e5dd636b..98d0b26f3 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -172,14 +172,16 @@ O(AssertStk, D(StkPtr), S(StkPtr), E) \ O(GuardRefs, ND, SUnk, E) \ O(AssertLoc, ND, S(FramePtr), E) \ O(OverrideLoc, ND, S(FramePtr), E) \ -O(OpAdd, DArith, SNum SNum, C) \ -O(OpSub, DArith, SNum SNum, C) \ -O(OpAnd, D(Int), SNumInt SNumInt, C) \ -O(OpOr, D(Int), SNum SNum, C) \ -O(OpXor, D(Int), SNumInt SNumInt, C) \ -O(OpMul, DArith, SNum SNum, C) \ -O(OpDiv, DArith, SNum SNum, C) \ -O(OpMod, D(Int), SNumInt SNumInt, C|N) \ +O(OpAdd, DArith, S(Int,Dbl) S(Int,Dbl), C) \ +O(OpSub, DArith, S(Int,Dbl) S(Int,Dbl), C) \ +O(OpMul, DArith, S(Int,Dbl) S(Int,Dbl), C) \ +O(OpDiv, DArith, S(Int,Dbl) S(Int,Dbl), C) \ +O(OpMod, DArith, S(Int,Dbl) S(Int,Dbl), C|N) \ +O(OpBitAnd, D(Int), S(Int) S(Int), C) \ +O(OpBitOr, D(Int), S(Int) S(Int), C) \ +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(ConvBoolToArr, D(Arr), S(Bool), C|N) \ @@ -589,13 +591,6 @@ enum Opcode : uint16_t { IR_OPCODES #undef O - // Agree with hhbc on the names of these, to ease macro implementations. - // TODO(2395841): this is totally bogus. Bitwise and logical ops have vastly - // different behavior. We need to give these their own opcodes. - OpBitAnd = OpAnd, - OpBitOr = OpOr, - OpBitXor = OpXor, - IR_NUM_OPCODES }; diff --git a/hphp/runtime/vm/translator/hopt/irtranslator.cpp b/hphp/runtime/vm/translator/hopt/irtranslator.cpp index 2eb883091..50de57cc8 100644 --- a/hphp/runtime/vm/translator/hopt/irtranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/irtranslator.cpp @@ -877,9 +877,9 @@ TranslatorX64::irTranslateSetOpL(const Tracelet& t, case SetOpDivEqual: HHIR_UNIMPLEMENTED(SetOpL_Div); case SetOpConcatEqual: opc = JIT::Concat; break; case SetOpModEqual: HHIR_UNIMPLEMENTED(SetOpL_Mod); - case SetOpAndEqual: opc = JIT::OpAnd; break; - case SetOpOrEqual: opc = JIT::OpOr; break; - case SetOpXorEqual: opc = JIT::OpXor; break; + case SetOpAndEqual: opc = JIT::OpBitAnd; break; + case SetOpOrEqual: opc = JIT::OpBitOr; break; + case SetOpXorEqual: opc = JIT::OpBitXor; break; case SetOpSlEqual: HHIR_UNIMPLEMENTED(SetOpL_Shl); case SetOpSrEqual: HHIR_UNIMPLEMENTED(SetOpL_Shr); default: not_reached(); diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index b76bcc1e3..e098f0b49 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -234,9 +234,10 @@ SSATmp* Simplifier::simplify(IRInstruction* inst) { case OpAdd: return simplifyAdd(src1, src2); case OpSub: return simplifySub(src1, src2); case OpMul: return simplifyMul(src1, src2); - case OpAnd: return simplifyAnd(src1, src2); - case OpOr: return simplifyOr(src1, src2); - case OpXor: return simplifyXor(src1, src2); + case OpBitAnd: return simplifyBitAnd(src1, src2); + case OpBitOr: return simplifyBitOr(src1, src2); + case OpBitXor: return simplifyBitXor(src1, src2); + case OpLogicXor: return simplifyLogicXor(src1, src2); case OpGt: case OpGte: @@ -866,13 +867,13 @@ SSATmp* Simplifier::simplifyMul(SSATmp* src1, SSATmp* src2) { return nullptr; } -SSATmp* Simplifier::simplifyAnd(SSATmp* src1, SSATmp* src2) { - SIMPLIFY_DISTRIBUTIVE(&, |, And, Or); +SSATmp* Simplifier::simplifyBitAnd(SSATmp* src1, SSATmp* src2) { + SIMPLIFY_DISTRIBUTIVE(&, |, BitAnd, BitOr); // X & X --> X if (src1 == src2) { return src1; } - if (src2->isConst() && src2->type() == Type::Int) { + if (src2->isConst()) { // X & 0 --> 0 if (src2->getValInt() == 0) { return cns(0); @@ -885,13 +886,13 @@ SSATmp* Simplifier::simplifyAnd(SSATmp* src1, SSATmp* src2) { return nullptr; } -SSATmp* Simplifier::simplifyOr(SSATmp* src1, SSATmp* src2) { - SIMPLIFY_DISTRIBUTIVE(|, &, Or, And); +SSATmp* Simplifier::simplifyBitOr(SSATmp* src1, SSATmp* src2) { + SIMPLIFY_DISTRIBUTIVE(|, &, BitOr, BitAnd); // X | X --> X if (src1 == src2) { return src1; } - if (src2->isConst() && src2->type() == Type::Int) { + if (src2->isConst()) { // X | 0 --> X if (src2->getValInt() == 0) { return src1; @@ -904,23 +905,37 @@ SSATmp* Simplifier::simplifyOr(SSATmp* src1, SSATmp* src2) { return nullptr; } -SSATmp* Simplifier::simplifyXor(SSATmp* src1, SSATmp* src2) { - SIMPLIFY_COMMUTATIVE(^, Xor); +SSATmp* Simplifier::simplifyBitXor(SSATmp* src1, SSATmp* src2) { + SIMPLIFY_COMMUTATIVE(^, BitXor); // X ^ X --> 0 if (src1 == src2) return cns(0); - // X ^ 0 --> X - if (src2->isConst() && src2->type() == Type::Int) { + // X ^ 0 --> X; X ^ -1 --> ~X + if (src2->isConst()) { if (src2->getValInt() == 0) { return src1; } + if (src2->getValInt() == -1) { + return gen(OpBitNot, src1); + } } - // Bool(X) ^ 1 --> Int(!X) - // Bool(X) ^ true --> Int(!X) - if (src1->isA(Type::Bool) && src2->isConst() && - ((src2->isA(Type::Int) && src2->getValInt() == 1) || - (src2->isA(Type::Bool) && src2->getValBool() == true))) { - return gen(ConvBoolToInt, gen(OpNot, src1)); + return nullptr; +} + +SSATmp* Simplifier::simplifyLogicXor(SSATmp* src1, SSATmp* src2) { + SIMPLIFY_COMMUTATIVE(^, LogicXor); + if (src1 == src2) { + return cns(false); + } + + // SIMPLIFY_COMMUTATIVE takes care of the both-sides-const case, and + // canonicalizes a single const to the right + if (src2->isConst()) { + if (src2->getValBool()) { + return gen(OpNot, src1); + } else { + return src1; + } } return nullptr; } diff --git a/hphp/runtime/vm/translator/hopt/simplifier.h b/hphp/runtime/vm/translator/hopt/simplifier.h index c8c02d9fa..b68cc5c5d 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.h +++ b/hphp/runtime/vm/translator/hopt/simplifier.h @@ -66,9 +66,10 @@ private: SSATmp* simplifyAdd(SSATmp* src1, SSATmp* src2); SSATmp* simplifySub(SSATmp* src1, SSATmp* src2); SSATmp* simplifyMul(SSATmp* src1, SSATmp* src2); - SSATmp* simplifyAnd(SSATmp* src1, SSATmp* src2); - SSATmp* simplifyOr(SSATmp* src1, SSATmp* src2); - SSATmp* simplifyXor(SSATmp* src1, SSATmp* src2); + SSATmp* simplifyBitAnd(SSATmp* src1, SSATmp* src2); + SSATmp* simplifyBitOr(SSATmp* src1, SSATmp* src2); + SSATmp* simplifyBitXor(SSATmp* src1, SSATmp* src2); + SSATmp* simplifyLogicXor(SSATmp* src1, SSATmp* src2); SSATmp* simplifyGt(SSATmp* src1, SSATmp* src2); SSATmp* simplifyGte(SSATmp* src1, SSATmp* src2); SSATmp* simplifyLt(SSATmp* src1, SSATmp* src2); diff --git a/hphp/runtime/vm/translator/hopt/type.cpp b/hphp/runtime/vm/translator/hopt/type.cpp index fc60b89a9..6d774703b 100644 --- a/hphp/runtime/vm/translator/hopt/type.cpp +++ b/hphp/runtime/vm/translator/hopt/type.cpp @@ -77,7 +77,14 @@ Type stkReturn(const IRInstruction* inst, int dstId, return Type::StkPtr; } -Type binArithResultType(Type t1, Type t2) { +Type binArithResultType(Opcode op, Type t1, Type t2) { + if (op == OpDiv) { + return Type::Int | Type::Dbl | Type::Bool; + } + if (op == OpMod) { + return Type::Int | Type::Bool; + } + assert(op == OpAdd || op == OpSub || op == OpMul); if (t1.subtypeOf(Type::Dbl) || t2.subtypeOf(Type::Dbl)) { return Type::Dbl; } @@ -99,7 +106,8 @@ Type outputType(const IRInstruction* inst, int dstId) { #define DVector return vectorReturn(inst); #define ND assert(0 && "outputType requires HasDest or NaryDest"); #define DBuiltin return builtinReturn(inst); -#define DArith return binArithResultType(inst->getSrc(0)->type(), \ +#define DArith return binArithResultType(inst->op(), \ + inst->getSrc(0)->type(), \ inst->getSrc(1)->type()); #define O(name, dstinfo, srcinfo, flags) case name: dstinfo not_reached();