diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 4bbccbe07..f68befba9 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -343,6 +343,11 @@ AssertLoc S0:FramePtr GuardLoc except it doesn't imply a runtime check and cannot cause control flow. +OverrideLoc S0:StkPtr + + Overrides tracked information about the type of a local in frame S0. + This is used to fix tracked state after InterpOne instructions. + D:StkPtr = GuardStk S0:StkPtr S1:ConstInt -> L Check that the type of the cell on the stack pointed to by S0 at diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index a56babaf0..6d99adfea 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -303,6 +303,7 @@ NOOP_OPCODE(DefFP) NOOP_OPCODE(DefSP) NOOP_OPCODE(Marker) NOOP_OPCODE(AssertLoc) +NOOP_OPCODE(OverrideLoc) NOOP_OPCODE(AssertStk) NOOP_OPCODE(Nop) NOOP_OPCODE(DefLabel) diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index b6c7ff51b..2fabe3dfe 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -889,7 +889,13 @@ void HhbcTranslator::emitCreateCont(bool getArgs, SSATmp* locals = gen(LdContLocalsPtr, cont); for (int i = 0; i < origLocals; ++i) { - SSATmp* loc = gen(IncRef, m_tb->genLdAssertedLoc(i, Type::Gen)); + // We must generate an AssertLoc because we don't have tracelet + // guards on the object type in these outer generator functions. + gen(AssertLoc, Type::Gen, LocalId(i), m_tb->getFp()); + auto const loc = gen( + IncRef, + m_tb->genLdLoc(i) + ); gen( StMem, locals, @@ -960,13 +966,15 @@ void HhbcTranslator::emitContExit() { void HhbcTranslator::emitUnpackCont() { gen(LinkContVarEnv, m_tb->getFp()); - SSATmp* cont = m_tb->genLdAssertedLoc(0, Type::Obj); + gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); + auto const cont = m_tb->genLdLoc(0); push(gen(LdRaw, Type::Int, cont, cns(RawMemSlot::ContLabel))); } void HhbcTranslator::emitPackCont(int64_t labelId) { gen(UnlinkContVarEnv, m_tb->getFp()); - SSATmp* cont = m_tb->genLdAssertedLoc(0, Type::Obj); + gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); + auto const cont = m_tb->genLdLoc(0); m_tb->genSetPropCell(cont, CONTOFF(m_value), popC()); gen( StRaw, cont, cns(RawMemSlot::ContLabel), cns(labelId) @@ -974,7 +982,8 @@ void HhbcTranslator::emitPackCont(int64_t labelId) { } void HhbcTranslator::emitContReceive() { - SSATmp* cont = m_tb->genLdAssertedLoc(0, Type::Obj); + gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); + auto const cont = m_tb->genLdLoc(0); gen(ContRaiseCheck, getExitSlowTrace(), cont); auto const valOffset = cns(CONTOFF(m_received)); push(gen(LdProp, Type::Cell, cont, valOffset)); @@ -982,7 +991,8 @@ void HhbcTranslator::emitContReceive() { } void HhbcTranslator::emitContRetC() { - SSATmp* cont = m_tb->genLdAssertedLoc(0, Type::Obj); + gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); + auto const cont = m_tb->genLdLoc(0); gen(ExitWhenSurprised, getExitSlowTrace()); gen( StRaw, cont, cns(RawMemSlot::ContDone), cns(true) @@ -1006,8 +1016,8 @@ void HhbcTranslator::emitContSendImpl(bool raise) { gen(ContStartedCheck, getExitSlowTrace(), cont); gen(ContPreNext, getExitSlowTrace(), cont); - SSATmp* value = m_tb->genLdAssertedLoc(0, Type::Cell); - value = gen(IncRef, value); + gen(AssertLoc, Type::Cell, LocalId(0), m_tb->getFp()); + auto const value = gen(IncRef, m_tb->genLdLoc(0)); m_tb->genSetPropCell(cont, CONTOFF(m_received), value); if (raise) { gen( @@ -2105,20 +2115,15 @@ void HhbcTranslator::guardTypeLocal(uint32_t locId, Type type) { } void HhbcTranslator::checkTypeLocal(uint32_t locId, Type type) { - m_tb->genGuardLoc(locId, type, getExitTrace()); + gen(GuardLoc, type, getExitTrace(), LocalId(locId), m_tb->getFp()); } -void HhbcTranslator::assertTypeLocal(uint32_t localIndex, Type type) { - m_tb->genAssertLoc(localIndex, type, false); +void HhbcTranslator::assertTypeLocal(uint32_t locId, Type type) { + gen(AssertLoc, type, LocalId(locId), m_tb->getFp()); } -void HhbcTranslator::overrideTypeLocal(uint32_t localIndex, Type type) { - // if changing the inner type of a boxed local, also drop the - // information about inner types for any other boxed local - if (type.isBoxed()) { - m_tb->dropLocalRefsInnerTypes(); - } - m_tb->genAssertLoc(localIndex, type, true); +void HhbcTranslator::overrideTypeLocal(uint32_t locId, Type type) { + gen(OverrideLoc, type, LocalId(locId), m_tb->getFp()); } Trace* HhbcTranslator::guardTypeStack(uint32_t stackIndex, diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 96faa95cd..a66e340df 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -172,6 +172,7 @@ O(CastStk, D(StkPtr), S(StkPtr) C(Int), Mem|N|Er) \ O(AssertStk, D(StkPtr), S(StkPtr) C(Int), 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) \ @@ -812,6 +813,7 @@ X(Marker, MarkerData); X(RaiseUninitLoc, LocalId); X(GuardLoc, LocalId); X(AssertLoc, LocalId); +X(OverrideLoc, LocalId); X(LdLocAddr, LocalId); X(DecRefLoc, LocalId); X(LdLoc, LocalId); diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index b394af115..55b1db75b 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -263,41 +263,6 @@ SSATmp* TraceBuilder::genCmp(Opcode opc, SSATmp* src1, SSATmp* src2) { return gen(opc, src1, src2); } -void TraceBuilder::genGuardLoc(uint32_t id, Type type, Trace* exitTrace) { - SSATmp* prevValue = getLocalValue(id); - if (prevValue) { - gen(GuardType, type, exitTrace, prevValue); - return; - } - Type prevType = getLocalType(id); - if (prevType == Type::None) { - gen(GuardLoc, type, getFirstBlock(exitTrace), LocalId(id), m_fpValue); - } else { - // It doesn't make sense to be guarding on something that's deemed to fail - assert(prevType == type); - } -} - -/** - * Generates an AssertLoc instruction for the given local 'id' and 'type'. - * If the 'override' flag is not set, then 'type' must be a subtype of the - * previous tracked type for this local. - */ -void TraceBuilder::genAssertLoc(uint32_t id, Type type, - bool overrideType /* =false */) { - Type prevType = overrideType ? Type::None : getLocalType(id); - if (prevType == Type::None || type.strictSubtypeOf(prevType)) { - gen(AssertLoc, type, LocalId(id), m_fpValue); - } else { - assert(prevType == type || prevType.strictSubtypeOf(type)); - } -} - -SSATmp* TraceBuilder::genLdAssertedLoc(uint32_t id, Type type) { - genAssertLoc(id, type); - return genLdLoc(id); -} - SSATmp* TraceBuilder::genBoxLoc(uint32_t id) { SSATmp* prevValue = genLdLoc(id); Type prevType = prevValue->type(); @@ -867,6 +832,13 @@ void TraceBuilder::updateTrackedState(IRInstruction* inst) { setLocalValue(inst->getExtra()->locId, inst->getDst()); break; + case OverrideLoc: + // If changing the inner type of a boxed local, also drop the + // information about inner types for any other boxed locals. + if (inst->getTypeParam().isBoxed()) { + dropLocalRefsInnerTypes(); + } + // fallthrough case AssertLoc: case GuardLoc: setLocalType(inst->getExtra()->locId, @@ -1162,6 +1134,39 @@ SSATmp* TraceBuilder::cseLookup(IRInstruction* inst) { ////////////////////////////////////////////////////////////////////// +SSATmp* TraceBuilder::preOptimizeGuardLoc(IRInstruction* inst) { + auto const locId = inst->getExtra()->locId; + + if (auto const prevValue = getLocalValue(locId)) { + return gen( + GuardType, inst->getTypeParam(), inst->getTaken(), prevValue + ); + } + + auto const prevType = getLocalType(locId); + if (prevType != Type::None) { + // It doesn't make sense to be guarding on something that's deemed + // to fail. + assert(prevType == inst->getTypeParam()); + inst->convertToNop(); + } + + return nullptr; +} + +SSATmp* TraceBuilder::preOptimizeAssertLoc(IRInstruction* inst) { + auto const locId = inst->getExtra()->locId; + auto const prevType = getLocalType(locId); + auto const typeParam = inst->getTypeParam(); + + if (prevType != Type::None && !typeParam.strictSubtypeOf(prevType)) { + assert(prevType.subtypeOf(typeParam)); + inst->convertToNop(); + } + + return nullptr; +} + SSATmp* TraceBuilder::preOptimizeLdThis(IRInstruction* inst) { if (isThisAvailable()) inst->setTaken(nullptr); return nullptr; @@ -1198,6 +1203,8 @@ SSATmp* TraceBuilder::preOptimizeDecRef(IRInstruction* inst) { SSATmp* TraceBuilder::preOptimize(IRInstruction* inst) { #define X(op) case op: return preOptimize##op(inst) switch (inst->op()) { + X(GuardLoc); + X(AssertLoc); X(LdThis); X(LdCtx); X(DecRef); diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.h b/hphp/runtime/vm/translator/hopt/tracebuilder.h index 970e9249f..3d606940c 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.h +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.h @@ -100,12 +100,6 @@ public: */ SSATmp* genLdLocAsCell(uint32_t id, Trace* exitTrace); - /* - * Asserts that local 'id' has type 'type' and loads it into the - * returned SSATmp. - */ - SSATmp* genLdAssertedLoc(uint32_t id, Type type); - SSATmp* genStLoc(uint32_t id, SSATmp* src, bool doRefCount, @@ -116,11 +110,7 @@ public: SSATmp* genBoxLoc(uint32_t id); void genBindLoc(uint32_t id, SSATmp* ref, bool doRefCount = true); - void genGuardLoc(uint32_t id, Type type, Trace* exitTrace); void genAssertStk(uint32_t id, Type type); - void genAssertLoc(uint32_t id, - Type type, - bool override = false); // ignores conflict w/ prev type // TODO(#2058865): we should have a real not opcode SSATmp* genNot(SSATmp* src); @@ -309,6 +299,8 @@ public: } private: + SSATmp* preOptimizeGuardLoc(IRInstruction*); + SSATmp* preOptimizeAssertLoc(IRInstruction*); SSATmp* preOptimizeLdThis(IRInstruction*); SSATmp* preOptimizeLdCtx(IRInstruction*); SSATmp* preOptimizeDecRef(IRInstruction*);