diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index e4fdf4b6e..8719c12bc 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -516,8 +516,7 @@ void shuffle2(CodeGenerator::Asm& a, } } -static void signExtendBool(X64Assembler& as, const SSATmp* tmp, - const RegisterInfo& info) { +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 @@ -525,15 +524,19 @@ static void signExtendBool(X64Assembler& as, const SSATmp* tmp, } } +static void zeroExtendBool(X64Assembler& as, const RegisterInfo& info) { + auto reg = info.getReg(); + if (reg != InvalidReg) { + // zero-extend the bool from a byte to a quad + // note: movzbl actually extends the value to 64 bits. + as.movzbl(rbyte(reg), r32(reg)); + } +} + static void zeroExtendIfBool(X64Assembler& as, const SSATmp* src, const RegisterInfo& info) { if (src->isA(Type::Bool)) { - auto reg = info.getReg(); - if (reg != InvalidReg) { - // zero-extend the bool from a byte to a quad - // note: movzbl actually extends the value to 64 bits. - as.movzbl(rbyte(reg), r32(reg)); - } + zeroExtendBool(as, info); } } @@ -979,7 +982,7 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, void (Asm::*instrIR)(Immed, RegType), void (Asm::*instrRR)(RegType, RegType), void (Asm::*movInstr)(RegType, RegType), - void (*extendInstr)(Asm&, const SSATmp*, const RegisterInfo&), + void (*extendInstr)(Asm&, const RegisterInfo&), Oper oper, RegType (*convertReg)(PhysReg), Commutativity commuteFlag) { @@ -1014,7 +1017,7 @@ void CodeGenerator::cgBinaryIntOp(IRInstruction* inst, auto signExtendIfNecessary = [&] { if (useBoolOps) { - extendInstr(m_as, dst, m_regs[dst]); + extendInstr(m_as, m_regs[dst]); } }; @@ -1093,7 +1096,7 @@ void CodeGenerator::cgBinaryOp(IRInstruction* inst, void (Asm::*instrRR)(RegType, RegType), void (Asm::*movInstr)(RegType, RegType), void (Asm::*fpInstr)(RegXMM, RegXMM), - void (*extendInstr)(Asm&, const SSATmp*, const RegisterInfo&), + void (*extendInstr)(Asm&, const RegisterInfo&), Oper oper, RegType (*convertReg)(PhysReg), Commutativity commuteFlag) { @@ -1154,13 +1157,12 @@ 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)) - { + if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { cgBinaryIntOp(inst, &Asm::addb, &Asm::addb, &Asm::movb, - &zeroExtendIfBool, + &zeroExtendBool, std::plus(), &convertToReg8, Commutative); @@ -1189,7 +1191,7 @@ void CodeGenerator::cgOpSub(IRInstruction* inst) { return cgNegateWork(dst, src2); } - if (src1->isA(Type::Bool) & src2->isA(Type::Bool)) { + if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { cgBinaryIntOp(inst, &Asm::subb, &Asm::subb, @@ -1215,12 +1217,12 @@ void CodeGenerator::cgOpAnd(IRInstruction* inst) { SSATmp* src1 = inst->getSrc(0); SSATmp* src2 = inst->getSrc(1); - if (src1->isA(Type::Bool) & src2->isA(Type::Bool)) { + if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { cgBinaryIntOp(inst, &Asm::andb, &Asm::andb, &Asm::movb, - &zeroExtendIfBool, + &zeroExtendBool, [] (bool a, bool b) { return a & b; }, &convertToReg8, Commutative); @@ -1240,12 +1242,12 @@ void CodeGenerator::cgOpOr(IRInstruction* inst) { SSATmp* src1 = inst->getSrc(0); SSATmp* src2 = inst->getSrc(1); - if (src1->isA(Type::Bool) & src2->isA(Type::Bool)) { + if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { cgBinaryIntOp(inst, &Asm::orb, &Asm::orb, &Asm::movb, - &zeroExtendIfBool, + &zeroExtendBool, [] (bool a, bool b) { return a | b; }, &convertToReg8, Commutative); @@ -1274,12 +1276,12 @@ void CodeGenerator::cgOpXor(IRInstruction* inst) { return cgNotWork(dst, src1); } - if (src1->isA(Type::Bool) & src2->isA(Type::Bool)) { + if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { cgBinaryIntOp(inst, &Asm::xorb, &Asm::xorb, &Asm::movb, - &zeroExtendIfBool, + &zeroExtendBool, [] (bool a, bool b) { return a ^ b; }, &convertToReg8, Commutative); @@ -1299,12 +1301,12 @@ void CodeGenerator::cgOpMul(IRInstruction* inst) { SSATmp* src1 = inst->getSrc(0); SSATmp* src2 = inst->getSrc(1); - if (src1->isA(Type::Bool) & src2->isA(Type::Bool)) { + if (src1->isA(Type::Bool) && src2->isA(Type::Bool)) { cgBinaryIntOp(inst, &Asm::andb, &Asm::andb, &Asm::movb, - &zeroExtendIfBool, + &zeroExtendBool, [] (bool a, bool b) { return a & b; }, &convertToReg8, Commutative); diff --git a/hphp/runtime/vm/translator/hopt/codegen.h b/hphp/runtime/vm/translator/hopt/codegen.h index 8e6454e33..176938806 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.h +++ b/hphp/runtime/vm/translator/hopt/codegen.h @@ -212,7 +212,7 @@ private: void (Asm::*intRR)(RegType, RegType), void (Asm::*mov)(RegType, RegType), void (Asm::*fpRR)(RegXMM, RegXMM), - void (*extend)(Asm&, const SSATmp*, const RegisterInfo&), + void (*extend)(Asm&, const RegisterInfo&), Oper, RegType (*conv)(PhysReg), Commutativity); @@ -221,7 +221,7 @@ private: void (Asm::*intImm)(Immed, RegType), void (Asm::*intRR)(RegType, RegType), void (Asm::*mov)(RegType, RegType), - void (*extend)(Asm&, const SSATmp*, const RegisterInfo&), + void (*extend)(Asm&, const RegisterInfo&), Oper, RegType (*conv)(PhysReg), Commutativity); diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 6e1ad46d2..14dbd7599 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -593,6 +593,8 @@ enum Opcode : uint16_t { #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, diff --git a/hphp/test/slow/types/1751.php b/hphp/test/slow/types/1751.php new file mode 100644 index 000000000..df44cada1 --- /dev/null +++ b/hphp/test/slow/types/1751.php @@ -0,0 +1,10 @@ +