diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 4ce998e7b..9d60f0466 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -452,7 +452,6 @@ D:Bool = OpNSame S0:Gen S1:Gen result in D. D:Bool = InstanceOf S0:Cls S1:Cls S2:ConstBool -D:Bool = NInstanceOf S0:Cls S1:Cls S2:ConstBool Sets D based on whether S0 is a descendant of the class, interface, or trait in S1. (Note that this is always false for a trait). S1 @@ -500,7 +499,6 @@ JmpNeq JmpSame JmpNSame JmpInstanceOf -JmpNInstanceOf JmpInstanceOfBitmask JmpNInstanceOfBitmask JmpIsType diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 09a91a6aa..089801411 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -1257,6 +1257,21 @@ void CodeGenerator::cgOpMul(IRInstruction* inst) { } } +void CodeGenerator::cgOpNot(IRInstruction* inst) { + auto const src = inst->getSrc(0); + auto const dstReg = inst->getDst()->getReg(); + auto& a = m_as; + + if (src->isConst()) { + a. movb (!src->getValBool(), rbyte(dstReg)); + } else { + if (dstReg != src->getReg()) { + a. movb (rbyte(src->getReg()), rbyte(dstReg)); + } + a. xorb (1, rbyte(dstReg)); + } +} + /////////////////////////////////////////////////////////////////////////////// // Comparison Operators /////////////////////////////////////////////////////////////////////////////// @@ -1599,46 +1614,17 @@ HOT_FUNC_VM static bool instanceOfHelperIFace(const Class* objClass, return testClass && objClass->classof(testClass->preClass()); } -void CodeGenerator::emitInstanceCheck(IRInstruction* inst, PhysReg dstReg) { +void CodeGenerator::cgInstanceOf(IRInstruction* inst) { const bool ifaceHint = inst->getSrc(2)->getValBool(); cgCallHelper(m_as, TCA(ifaceHint ? instanceOfHelperIFace : instanceOfHelper), - dstReg, + inst->getDst(), kNoSyncPoint, ArgGroup() .ssa(inst->getSrc(0)) .ssa(inst->getSrc(1))); } -void CodeGenerator::cgInstanceOf(IRInstruction* inst) { - emitInstanceCheck(inst, inst->getDst()->getReg()); -} - -void CodeGenerator::cgNInstanceOf(IRInstruction* inst) { - // TODO(#2058865): having NInstanceOf is no better than InstanceOf - // followed by boolean Not opcode. - PhysReg dstReg = inst->getDst()->getReg(); - emitInstanceCheck(inst, dstReg); - Reg8 dr((int(dstReg))); - auto& a = m_as; - a. testb (dr, dr); - a. setz (dr); -} - -void CodeGenerator::cgJmpInstanceOf(IRInstruction* inst) { - auto& a = m_as; - emitInstanceCheck(inst, rax); - a. testb (al, al); - emitJccDirectExit(inst, CC_NZ); -} - -void CodeGenerator::cgJmpNInstanceOf(IRInstruction* inst) { - auto& a = m_as; - emitInstanceCheck(inst, rax); - a. testb (al, al); - emitJccDirectExit(inst, CC_Z); -} - /* * Check instanceof using instance bitmasks. * diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index 5120d9620..b09a10b0c 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -1051,7 +1051,7 @@ void HhbcTranslator::emitContValid() { SSATmp* done = gen( LdRaw, Type::Bool, cont, cns(RawMemSlot::ContDone) ); - push(m_tb->genNot(done)); + push(gen(OpNot, done)); } void HhbcTranslator::emitContCurrent() { @@ -1219,7 +1219,7 @@ void HhbcTranslator::emitEmptyL(int32_t id) { } else { Trace* exitTrace = getExitTrace(); SSATmp* ld = m_tb->genLdLocAsCell(id, exitTrace); - push(m_tb->genNot(gen(ConvCellToBool, ld))); + push(gen(OpNot, gen(ConvCellToBool, ld))); } } @@ -2653,7 +2653,7 @@ void HhbcTranslator::emitIsset(const StringData* name, void HhbcTranslator::emitEmptyMem(SSATmp* ptr) { SSATmp* ld = gen(LdMem, Type::Cell, gen(UnboxPtr, ptr), cns(0)); - push(m_tb->genNot(gen(ConvCellToBool, ld))); + push(gen(OpNot, gen(ConvCellToBool, ld))); } template @@ -2673,7 +2673,7 @@ void HhbcTranslator::emitEmpty(const StringData* name, gen(UnboxPtr, ptr), cns(0) ); - return m_tb->genNot(gen(ConvCellToBool, ld)); + return gen(OpNot, gen(ConvCellToBool, ld)); }, [&] { // Taken return cns(true); @@ -2796,7 +2796,7 @@ void HhbcTranslator::emitBinaryArith(Opcode opc) { void HhbcTranslator::emitNot() { SSATmp* src = popC(); - push(m_tb->genNot(gen(ConvCellToBool, src))); + push(gen(OpNot, gen(ConvCellToBool, src))); gen(DecRef, src); } diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index c84a4f7e2..b07316f76 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -547,8 +547,6 @@ Opcode negateQueryOp(Opcode opc) { case OpNeq: return OpEq; case OpSame: return OpNSame; case OpNSame: return OpSame; - case InstanceOf: return NInstanceOf; - case NInstanceOf: return InstanceOf; case InstanceOfBitmask: return NInstanceOfBitmask; case NInstanceOfBitmask: return InstanceOfBitmask; case IsType: return IsNType; diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 6de4fb7a7..dbb5ea455 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -181,6 +181,7 @@ 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(OpNot, D(Bool), S(Bool), C) \ \ O(ConvBoolToArr, D(Arr), S(Bool), C|N) \ O(ConvDblToArr, D(Arr), S(Dbl), C|N) \ @@ -218,6 +219,7 @@ O(ConvObjToStr, D(Str), S(Obj), N|Er|CRc|K) \ O(ConvCellToStr, D(Str), S(Cell), N|Er|CRc|K) \ \ O(ExtendsClass, D(Bool), S(Cls) C(Cls), C) \ +O(InstanceOf, D(Bool), S(Cls) S(Cls) C(Bool), C|N) \ O(IsTypeMem, D(Bool), S(PtrToGen), NA) \ O(IsNTypeMem, D(Bool), S(PtrToGen), NA) \ \ @@ -230,8 +232,6 @@ O(OpEq, D(Bool), S(Gen) S(Gen), C|N) \ O(OpNeq, D(Bool), S(Gen) S(Gen), C|N) \ O(OpSame, D(Bool), S(Gen) S(Gen), C|N) \ O(OpNSame, D(Bool), S(Gen) S(Gen), C|N) \ -O(InstanceOf, D(Bool), S(Cls) S(Cls) C(Bool), C|N) \ -O(NInstanceOf, D(Bool), S(Cls) S(Cls) C(Bool), C|N) \ O(InstanceOfBitmask, D(Bool), S(Cls) CStr, C) \ O(NInstanceOfBitmask, D(Bool), S(Cls) CStr, C) \ O(IsType, D(Bool), S(Cell), C) \ @@ -245,8 +245,6 @@ O(JmpEq, D(None), S(Gen) S(Gen), E) \ O(JmpNeq, D(None), S(Gen) S(Gen), E) \ O(JmpSame, D(None), S(Gen) S(Gen), E) \ O(JmpNSame, D(None), S(Gen) S(Gen), E) \ -O(JmpInstanceOf, D(None), S(Cls) S(Cls) C(Bool), E|N) \ -O(JmpNInstanceOf, D(None), S(Cls) S(Cls) C(Bool), E|N) \ O(JmpInstanceOfBitmask, D(None), S(Cls) CStr, E) \ O(JmpNInstanceOfBitmask, D(None), S(Cls) CStr, E) \ O(JmpIsType, D(None), SUnk, E) \ @@ -905,8 +903,6 @@ inline bool isQueryJmpOp(Opcode opc) { case JmpNeq: case JmpSame: case JmpNSame: - case JmpInstanceOf: - case JmpNInstanceOf: case JmpInstanceOfBitmask: case JmpNInstanceOfBitmask: case JmpIsType: @@ -939,8 +935,6 @@ inline ConditionCode queryJmpToCC(Opcode opc) { case JmpNeq: return CC_NE; case JmpSame: return CC_E; case JmpNSame: return CC_NE; - case JmpInstanceOf: return CC_NZ; - case JmpNInstanceOf: return CC_Z; case JmpInstanceOfBitmask: return CC_NZ; case JmpNInstanceOfBitmask: return CC_Z; case JmpIsType: return CC_NZ; @@ -952,14 +946,6 @@ inline ConditionCode queryJmpToCC(Opcode opc) { } } -/* - * Right now branch fusion is too indiscriminate to handle fusing - * with potentially expensive-to-repeat operations. TODO(#2053369) - */ -inline bool disableBranchFusion(Opcode opc) { - return opc == InstanceOf || opc == NInstanceOf; -} - /* * Return the opcode that corresponds to negation of opc. */ diff --git a/hphp/runtime/vm/translator/hopt/linearscan.cpp b/hphp/runtime/vm/translator/hopt/linearscan.cpp index 9d21ed8c3..3447e4781 100644 --- a/hphp/runtime/vm/translator/hopt/linearscan.cpp +++ b/hphp/runtime/vm/translator/hopt/linearscan.cpp @@ -740,9 +740,6 @@ void LinearScan::computePreColoringHint() { } break; case InstanceOf: - case NInstanceOf: - case JmpInstanceOf: - case JmpNInstanceOf: normalHint(2); break; case LdSSwitchDestFast: diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index c39819ffb..3e93eb30a 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -226,27 +226,6 @@ template SSATmp* Simplifier::gen(Args&&... args) { ////////////////////////////////////////////////////////////////////// -static bool isNotInst(SSATmp *src1, SSATmp *src2) { - // right operand should be 1 - if (!src2->isConst() || src2->type() != Type::Int || - src2->getValInt() != 1) { - return false; - } - // left operand should be a boolean - if (src1->type() != Type::Bool) { - return false; - } - return true; -} - -static bool isNotInst(SSATmp *tmp) { - IRInstruction* inst = tmp->inst(); - if (inst->op() != OpXor) { - return false; - } - return isNotInst(inst->getSrc(0), inst->getSrc(1)); -} - SSATmp* Simplifier::simplify(IRInstruction* inst) { SSATmp* src1 = inst->getSrc(0); SSATmp* src2 = inst->getSrc(1); @@ -272,6 +251,7 @@ SSATmp* Simplifier::simplify(IRInstruction* inst) { case Concat: return simplifyConcat(src1, src2); case Mov: return simplifyMov(src1); + case OpNot: return simplifyNot(src1); case LdClsPropAddr: return simplifyLdClsPropAddr(inst); case ConvBoolToArr: return simplifyConvToArr(inst); case ConvDblToArr: return simplifyConvToArr(inst); @@ -580,52 +560,45 @@ SSATmp* Simplifier::simplifyMov(SSATmp* src) { } SSATmp* Simplifier::simplifyNot(SSATmp* src) { + if (src->isConst()) { + return cns(!src->getValBool()); + } + IRInstruction* inst = src->inst(); Opcode op = inst->op(); - // TODO: Add more algebraic simplification rules for NOT switch (op) { - case ConvArrToBool: - case ConvDblToBool: - case ConvIntToBool: - case ConvStrToBool: - return simplifyNot(inst->getSrc(0)); - case OpXor: { - // !!X --> bool(X) - if (isNotInst(inst->getSrc(0))) { - return gen(ConvCellToBool, inst->getSrc(0)); - } - break; - } - // !(X cmp Y) --> X opposite_cmp Y - case OpLt: - case OpLte: - case OpGt: - case OpGte: - case OpEq: - case OpNeq: - case OpSame: - case OpNSame: - // XXX: this could technically be losing a ConvToBool, except - // that we kinda know "not" instructions (Xor with 1) are always - // going to be followed by ConvToBool. - // - // TODO(#2058865): This would make more sense with a real Not - // instruction and allowing boolean output types for query ops. + // !!X --> X + case OpNot: + return inst->getSrc(0); + + // !(X cmp Y) --> X opposite_cmp Y + case OpLt: + case OpLte: + case OpGt: + case OpGte: + case OpEq: + case OpNeq: + case OpSame: + case OpNSame: + // Not for Dbl: (x < NaN) != !(x >= NaN) + if (!inst->getSrc(0)->isA(Type::Dbl) && + !inst->getSrc(1)->isA(Type::Dbl)) { return gen(negateQueryOp(op), inst->getSrc(0), inst->getSrc(1)); - case InstanceOf: - case NInstanceOf: - case InstanceOfBitmask: - case NInstanceOfBitmask: - // TODO: combine this with the above check and use isQueryOp or - // add an isNegatable. - return gen( - negateQueryOp(op), - std::make_pair(inst->getNumSrcs(), inst->getSrcs().begin()) - ); - return nullptr; - // TODO !(X | non_zero) --> 0 - default: (void)op; + } + break; + + case InstanceOfBitmask: + case NInstanceOfBitmask: + // TODO: combine this with the above check and use isQueryOp or + // add an isNegatable. + return gen( + negateQueryOp(op), + std::make_pair(inst->getNumSrcs(), inst->getSrcs().begin()) + ); + return nullptr; + // TODO !(X | non_zero) --> 0 + default: (void)op; } return nullptr; } @@ -816,6 +789,7 @@ SSATmp* Simplifier::simplifyAdd(SSATmp* src1, SSATmp* src2) { } return nullptr; } + SSATmp* Simplifier::simplifySub(SSATmp* src1, SSATmp* src2) { SIMPLIFY_CONST(-); // X - X --> 0 @@ -853,6 +827,7 @@ SSATmp* Simplifier::simplifySub(SSATmp* src1, SSATmp* src2) { // (X - C1) + (X + C2) return nullptr; } + SSATmp* Simplifier::simplifyMul(SSATmp* src1, SSATmp* src2) { SIMPLIFY_COMMUTATIVE(*, Mul); if (src2->isConst() && src2->type() == Type::Int) { @@ -882,6 +857,7 @@ SSATmp* Simplifier::simplifyMul(SSATmp* src1, SSATmp* src2) { } return nullptr; } + SSATmp* Simplifier::simplifyAnd(SSATmp* src1, SSATmp* src2) { SIMPLIFY_DISTRIBUTIVE(&, |, And, Or); // X & X --> X @@ -900,6 +876,7 @@ SSATmp* Simplifier::simplifyAnd(SSATmp* src1, SSATmp* src2) { } return nullptr; } + SSATmp* Simplifier::simplifyOr(SSATmp* src1, SSATmp* src2) { SIMPLIFY_DISTRIBUTIVE(|, &, Or, And); // X | X --> X @@ -918,6 +895,7 @@ SSATmp* Simplifier::simplifyOr(SSATmp* src1, SSATmp* src2) { } return nullptr; } + SSATmp* Simplifier::simplifyXor(SSATmp* src1, SSATmp* src2) { SIMPLIFY_COMMUTATIVE(^, Xor); // X ^ X --> 0 @@ -929,8 +907,12 @@ SSATmp* Simplifier::simplifyXor(SSATmp* src1, SSATmp* src2) { return src1; } } - if (isNotInst(src1, src2)) { - return simplifyNot(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; } @@ -1424,11 +1406,7 @@ SSATmp* Simplifier::simplifyConvCellToBool(IRInstruction* inst) { if (srcType.isDbl()) return gen(ConvDblToBool, src); if (srcType.isInt()) return gen(ConvIntToBool, src); if (srcType.isString()) return gen(ConvStrToBool, src); - - if (srcType.isObj()) { - // If a value is known to be an object, it is known to be non null. - return cns(true); - } + if (srcType.isObj()) return cns(true); return nullptr; } @@ -1574,7 +1552,7 @@ SSATmp* Simplifier::simplifyCondJmp(IRInstruction* inst) { } // Pull negations into the jump. - if (isNotInst(src)) { + if (src->inst()->op() == OpNot) { return gen(inst->op() == JmpZero ? JmpNZero : JmpZero, inst->getTaken(), srcInst->getSrc(0)); @@ -1597,7 +1575,7 @@ SSATmp* Simplifier::simplifyCondJmp(IRInstruction* inst) { } // Fuse jumps with query operators. - if (isQueryOp(srcOpcode) && !disableBranchFusion(srcOpcode)) { + if (isQueryOp(srcOpcode)) { SrcRange ssas = srcInst->getSrcs(); return gen( queryToJmpOp( diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index 5d0f9c6ca..538a864c9 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -202,11 +202,6 @@ Trace* TraceBuilder::genExitTrace(uint32_t bcOff, return exitTrace; } -SSATmp* TraceBuilder::genNot(SSATmp* src) { - assert(src->type() == Type::Bool); - return gen(ConvCellToBool, gen(OpXor, src, cns(1))); -} - SSATmp* TraceBuilder::genDefUninit() { return gen(DefConst, Type::Uninit, ConstData(0)); } diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.h b/hphp/runtime/vm/translator/hopt/tracebuilder.h index 2cf2f5a45..2a4a3c5d1 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.h +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.h @@ -227,12 +227,6 @@ struct TraceBuilder { return gen(LdConst, typeForConst(val), ConstData(val)); } - ////////////////////////////////////////////////////////////////////// - // dubious - - // TODO(#2058865): we should have a real not opcode - SSATmp* genNot(SSATmp* src); - ////////////////////////////////////////////////////////////////////// // control flow