diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 42f785d51..b057fdcd2 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -446,7 +446,7 @@ D:CountedStr = AssertNonNull S0:{Nullptr|CountedStr} 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 = OpMod S0:Int S1:Int 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 @@ -455,9 +455,12 @@ 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. - Note that OpMod and OpDiv produce boolean false when the divisor is - zero-like. Ints are not closed under OpDiv, as OpDiv produces doubles - when there is a non-zero remainder. + Undefined behavior occurs if OpMod is given a divisor of zero, or if + the divisor is -1 and the dividend is the minimum representable integer. + + OpDiv produces boolean false when the divisor is 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 diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index 93db583c0..454adc378 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/hphp/runtime/vm/jit/codegen.cpp @@ -418,7 +418,6 @@ CALL_OPCODE(RaiseWarning) CALL_OPCODE(IncStatGrouped) CALL_OPCODE(StaticLocInit) CALL_OPCODE(StaticLocInitCached) -CALL_OPCODE(OpMod) CALL_OPCODE(ArrayIdx) // Vector instruction helpers @@ -1486,6 +1485,49 @@ void CodeGenerator::cgOpMul(IRInstruction* inst) { Commutative); } +void CodeGenerator::cgOpMod(IRInstruction* inst) { + auto const src0 = inst->src(0); + auto const src1 = inst->src(1); + auto const dstReg = m_regs[inst->dst()].reg(); + auto& a = m_as; + + // spill rax and/or rdx + bool spillRax = dstReg != reg::rax && m_rScratch != reg::rax; + bool spillRdx = dstReg != reg::rdx && m_rScratch != reg::rdx; + if (spillRax) { + a. push (reg::rax); + } + if (spillRdx) { + a. push (reg::rdx); + } + // put divisor in rAsm + if (src1->isConst()) { + a. movq (src1->getValInt(), rAsm); + } else { + a. movq (m_regs[src1].reg(), rAsm); + } + // put dividend in rax + if (src0->isConst()) { + a. movq (src0->getValInt(), reg::rax); + } else if (m_regs[src0].reg() != reg::rax) { + a. movq (m_regs[src0].reg(), reg::rax); + } + // sign-extend rax to rdx:rax + a. cqo (); + // divide + a. idiv (rAsm); + if (dstReg != reg::rdx) { + a. movq (reg::rdx, dstReg); + } + // restore rax and/or rdx + if (spillRdx) { + a. pop (reg::rdx); + } + if (spillRax) { + a. pop (reg::rax); + } +} + void CodeGenerator::cgOpNot(IRInstruction* inst) { auto const src = inst->src(0); auto const dstReg = m_regs[inst->dst()].reg(); diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp index f592b21d0..f89e97a9a 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.cpp +++ b/hphp/runtime/vm/jit/hhbctranslator.cpp @@ -3171,20 +3171,13 @@ void HhbcTranslator::emitDiv() { } void HhbcTranslator::emitMod() { - // XXX: Disabled until t2299606 is fixed - PUNT(emitMod); - - auto tl = topC(1)->type(); - auto tr = topC(0)->type(); - auto isInty = [&](Type t) { - return t.subtypeOf(Type::Null | Type::Bool | Type::Int); - }; - if (!(isInty(tl) && isInty(tr))) { - emitInterpOne(Type::Cell, 2); - return; - } - SSATmp* r = popC(); - SSATmp* l = popC(); + IRTrace* catchTrace = getCatchTrace(); + SSATmp* btr = popC(); + SSATmp* btl = popC(); + SSATmp* tr = gen(ConvCellToInt, catchTrace, btr); + SSATmp* tl = gen(ConvCellToInt, catchTrace, btl); + gen(DecRef, btr); + gen(DecRef, btl); // Exit path spills an additional false auto exitSpillValues = peekSpillValues(); exitSpillValues.push_back(cns(false)); @@ -3197,8 +3190,38 @@ void HhbcTranslator::emitMod() { exitSpillValues, StringData::GetStaticString(Strings::DIVISION_BY_ZERO) ); - gen(JmpZero, exit, r); - push(gen(OpMod, l, r)); + gen(JmpZero, exit, tr); + + // We unfortunately need to special-case r = -1 here. In two's + // complement, trying to divide INT_MIN by -1 will cause an integer + // overflow. + if (tr->isConst()) { + // This crap only exists so m_tb->cond doesn't get mad when one + // of the branches gets optimized out due to constant folding. + if (tr->getValInt() == -1LL) { + push(cns(0)); + } else { + push(gen(OpMod, tl, tr)); + } + return; + } + + // check for -1 (dynamic version) + SSATmp *res = m_tb->cond( + curFunc(), + [&] (Block* taken) { + SSATmp* negone = gen(OpEq, tr, cns(-1LL)); + gen(JmpNZero, taken, negone); + }, + [&] { + return gen(OpMod, tl, tr); + }, + [&] { + m_tb->hint(Block::Hint::Unlikely); + return cns(0); + } + ); + push(res); } void HhbcTranslator::emitBitNot() { diff --git a/hphp/runtime/vm/jit/ir.h b/hphp/runtime/vm/jit/ir.h index bfbead103..6d11112e5 100644 --- a/hphp/runtime/vm/jit/ir.h +++ b/hphp/runtime/vm/jit/ir.h @@ -211,7 +211,7 @@ 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(OpMod, D(Int), S(Int) S(Int), C) \ 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) \ diff --git a/hphp/runtime/vm/jit/nativecalls.cpp b/hphp/runtime/vm/jit/nativecalls.cpp index 7cb65d33c..53727557d 100644 --- a/hphp/runtime/vm/jit/nativecalls.cpp +++ b/hphp/runtime/vm/jit/nativecalls.cpp @@ -145,7 +145,6 @@ static CallMap s_callMap({ {VerifyParamFail, (TCA)VerifyParamTypeFail, DNone, SSync, {{SSA, 0}}}, {RaiseUninitLoc, (TCA)raiseUndefVariable, DNone, SSync, {{SSA, 0}}}, {RaiseWarning, (TCA)raiseWarning, DNone, SSync, {{SSA, 0}}}, - {OpMod, (TCA)modHelper, DSSA, SNone, {{SSA, 0}, {SSA, 1}}}, {WarnNonObjProp, (TCA)raisePropertyOnNonObject, DNone, SSync, {}}, {ThrowNonObjProp, (TCA)throw_null_object_prop, DNone, SSync, {}}, {RaiseUndefProp, (TCA)raiseUndefProp, DNone, SSync, diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp index 252ff8d6a..2919f5baf 100644 --- a/hphp/runtime/vm/jit/translator.cpp +++ b/hphp/runtime/vm/jit/translator.cpp @@ -1008,7 +1008,7 @@ static const struct { { OpMul, {StackTop2, Stack1, OutArith, -1 }}, /* Div and mod might return boolean false. Sigh. */ { OpDiv, {StackTop2, Stack1, OutUnknown, -1 }}, - { OpMod, {StackTop2, Stack1, OutUnknown, -1 }}, + { OpMod, {StackTop2, Stack1, OutPred, -1 }}, /* Logical ops */ { OpXor, {StackTop2, Stack1, OutBoolean, -1 }}, { OpNot, {Stack1, Stack1, OutBoolean, 0 }}, diff --git a/hphp/runtime/vm/jit/type.cpp b/hphp/runtime/vm/jit/type.cpp index eb4987643..39461ee7a 100644 --- a/hphp/runtime/vm/jit/type.cpp +++ b/hphp/runtime/vm/jit/type.cpp @@ -174,7 +174,7 @@ Type binArithResultType(Opcode op, Type t1, Type t2) { return Type::Int | Type::Dbl | Type::Bool; } if (op == OpMod) { - return Type::Int | Type::Bool; + return Type::Int; } assert(op == OpAdd || op == OpSub || op == OpMul); if (t1.subtypeOf(Type::Dbl) || t2.subtypeOf(Type::Dbl)) { diff --git a/hphp/util/asm-x64.h b/hphp/util/asm-x64.h index 1428470db..4fcea6644 100644 --- a/hphp/util/asm-x64.h +++ b/hphp/util/asm-x64.h @@ -758,6 +758,7 @@ const X64Instr instr_movsbx = { { 0xBE,0xF1,0xF1,0x00,0xF1,0xF1 }, 0x2003 }; const X64Instr instr_movzwx = { { 0xB7,0xF1,0xF1,0x00,0xF1,0xF1 }, 0x0003 }; const X64Instr instr_movzbx = { { 0xB6,0xF1,0xF1,0x00,0xF1,0xF1 }, 0x2003 }; const X64Instr instr_cwde = { { 0xF1,0xF1,0xF1,0x00,0xF1,0x98 }, 0x0400 }; +const X64Instr instr_cqo = { { 0xF1,0xF1,0xF1,0x00,0xF1,0x99 }, 0x0000 }; const X64Instr instr_rol = { { 0xD3,0xF1,0xC1,0x00,0xF1,0xF1 }, 0x0020 }; const X64Instr instr_ror = { { 0xD3,0xF1,0xC1,0x01,0xF1,0xF1 }, 0x0020 }; const X64Instr instr_rcl = { { 0xD3,0xF1,0xC1,0x02,0xF1,0xF1 }, 0x0020 }; @@ -1098,6 +1099,7 @@ struct X64Assembler { void negb(Reg8 r) { instrR(instr_negb, r); } void ret() { emit(instr_ret); } void ret(Immed i) { emitI(instr_ret, i.w(), sz::word); } + void cqo() { emit(instr_cqo); } void nop() { emit(instr_nop); } void int3() { emit(instr_int3); } void ud2() { byte(0x0f); byte(0x0b); }