diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 60db73c50..d271098f7 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -343,7 +343,7 @@ AssertLoc S0:FramePtr GuardLoc except it doesn't imply a runtime check and cannot cause control flow. -OverrideLoc S0:StkPtr +OverrideLoc S0:FramePtr Overrides tracked information about the type of a local in frame S0. This is used to fix tracked state after InterpOne instructions. @@ -990,8 +990,18 @@ StLocNT S0:FramePtr S1:Gen Store S1 to local number localId on the frame pointed to by S0, without storing the type. -StRef -StRefNT +D:BoxedCell = StRef S0:BoxedCell S1:Cell + + Store the value in S1 into the RefData pointed to by S0. Stores the + RefData::m_type also. Returns a new value for S0, which will now + have the type of S1 after boxing. + +D:BoxedCell = StRefNT S0:BoxedCell S1:Cell + + Store the value in S1 into the RefData pointed to by S0. Does not + change RefData::m_type. Returns a new value for S0, which will now + have the type of S1 after boxing. + StRaw D:StkPtr = SpillStack S0:StkP S1:ConstInt, S2... diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index d78e3a497..0ebdbdbac 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -1978,7 +1978,6 @@ void CodeGenerator::cgLdFuncCached(IRInstruction* inst) { }); } - void CodeGenerator::cgLdFuncCachedSafe(IRInstruction* inst) { cgLdFuncCachedCommon(inst); if (Block* taken = inst->getTaken()) { diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index dad47c984..5cbb63713 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -605,18 +605,26 @@ void HhbcTranslator::emitInitThisLoc(int32_t id) { void HhbcTranslator::emitCGetL(int32_t id) { Trace* exitTrace = getExitTrace(); - pushIncRef(emitLdLocWarn(id, exitTrace)); + pushIncRef(ldLocInnerWarn(id, exitTrace)); } void HhbcTranslator::emitCGetL2(int32_t id) { Trace* exitTrace = getExitTrace(); SSATmp* oldTop = pop(Type::Gen); - pushIncRef(emitLdLocWarn(id, exitTrace)); + pushIncRef(ldLocInnerWarn(id, exitTrace)); push(oldTop); } void HhbcTranslator::emitVGetL(int32_t id) { - pushIncRef(m_tb->genBoxLoc(id)); + auto value = ldLoc(id); + if (!value->type().isBoxed()) { + if (value->isA(Type::Uninit)) { + value = m_tb->genDefInitNull(); + } + value = gen(Box, value); + gen(StLoc, LocalId(id), m_tb->getFp(), value); + } + pushIncRef(value); } void HhbcTranslator::emitUnsetN() { @@ -630,45 +638,49 @@ void HhbcTranslator::emitUnsetG(const StringData* gblName) { } void HhbcTranslator::emitUnsetL(int32_t id) { - m_tb->genBindLoc(id, m_tb->genDefUninit()); + auto const prev = ldLoc(id); + gen(StLoc, LocalId(id), m_tb->getFp(), m_tb->genDefUninit()); + gen(DecRef, prev); } void HhbcTranslator::emitBindL(int32_t id) { - SSATmp* src = popV(); + auto const newValue = popV(); + // XXX: does this cause user-visible semantic differences? What + // about the stack being different when we re-enter the dtor? What + // is this for? if (getCurFunc()->isPseudoMain()) { // in pseudo mains, the value of locals could change in functions // called explicitly (or implicitly via exceptions or destructors) // so we need to incref eagerly in case one of these functions // changes the value of our local and makes src dead. - pushIncRef(src); + pushIncRef(newValue); } - m_tb->genBindLoc(id, src); + auto const oldValue = ldLoc(id); + gen(StLoc, LocalId(id), m_tb->getFp(), newValue); + gen(DecRef, oldValue); if (!getCurFunc()->isPseudoMain()) { - pushIncRef(src); + pushIncRef(newValue); } } void HhbcTranslator::emitSetL(int32_t id) { - Trace* exitTrace = getExitTrace(); - SSATmp* src = popC(); - // Note we can't use the same trick as emitBindL in which we - // move the incref to after the store because the stored location - // might contain a ref, which could then be modified by the decref - // inserted after the stloc - push(m_tb->genStLoc(id, src, true, true, exitTrace)); + auto const exitTrace = getExitTrace(); + auto const src = popC(); + push(stLoc(id, exitTrace, src)); } void HhbcTranslator::emitIncDecL(bool pre, bool inc, uint32_t id) { - // Handle only integer inc/dec for now Trace* exitTrace = getExitTrace(); - SSATmp* src = m_tb->genLdLocAsCell(id, exitTrace); + auto const src = ldLocInner(id, exitTrace); + + // Inc/Dec of a bool is a no-op. if (src->isA(Type::Bool)) { - // inc/dec of a bool is a no-op push(src); - } else { - SSATmp* res = emitIncDec(pre, inc, src); - m_tb->genStLoc(id, res, false, false, nullptr); + return; } + + auto const res = emitIncDec(pre, inc, src); + stLoc(id, exitTrace, res); } // only handles integer or double inc/dec @@ -693,41 +705,43 @@ void HhbcTranslator::emitIncDecMem(bool pre, gen(StMemNT, propAddr, cns(0), res); } -static bool isSupportedBinaryArith(Opcode opc, - Type t1, - Type t2) { +static bool isSupportedBinaryArith(Opcode opc, Type t1, Type t2) { switch (opc) { - // Opcodes supporting FP - case OpAdd: - case OpSub: - case OpMul: return (t1.subtypeOf(Type::Int | Type::Bool | Type::Dbl) && - t2.subtypeOf(Type::Int | Type::Bool | Type::Dbl)); + 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.subtypeOf(Type::Int | Type::Bool) && - t2.subtypeOf(Type::Int | Type::Bool)); + default: return t1.subtypeOfAny(Type::Int, Type::Bool) && + t2.subtypeOfAny(Type::Int, Type::Bool); } } void HhbcTranslator::emitSetOpL(Opcode subOpc, uint32_t id) { - Trace* exitTrace = getExitTrace(); - SSATmp* loc = emitLdLocWarn(id, exitTrace); + auto const exitTrace = getExitTrace(); + auto const loc = ldLocInnerWarn(id, exitTrace); + if (subOpc == Concat) { - // The concat helpers decref their args, so don't decref pop'ed values - // and don't decref the old value held in the local. The concat helpers - // also incref their results, which will be consumed by the stloc. We - // need an extra incref for the push onto the stack. - SSATmp* val = popC(); - SSATmp* result = gen(Concat, loc, val); - pushIncRef(m_tb->genStLoc(id, result, false, true, exitTrace)); - } else if (isSupportedBinaryArith(subOpc, - loc->type(), - topC()->type())) { - SSATmp* val = popC(); - SSATmp* result = gen(subOpc, loc, val); - push(m_tb->genStLoc(id, result, true, true, exitTrace)); - } else { - PUNT(SetOpL); + /* + * The concat helpers decref their args, so don't decref pop'ed values + * and don't decref the old value held in the local. The concat helpers + * also incref their results, which will be consumed by the stloc. We + * need an extra incref for the push onto the stack. + */ + auto const val = popC(); + auto const result = gen(Concat, loc, val); + pushIncRef(stLocNRC(id, exitTrace, result)); + return; } + + if (isSupportedBinaryArith(subOpc, loc->type(), topC()->type())) { + auto const val = popC(); + auto const result = gen(subOpc, loc, val); + push(stLoc(id, exitTrace, result)); + return; + } + + PUNT(SetOpL); } void HhbcTranslator::emitClassExists(const StringData* clsName) { @@ -891,10 +905,7 @@ void HhbcTranslator::emitCreateCont(bool getArgs, // 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) - ); + auto const loc = gen(IncRef, ldLoc(i)); gen( StMem, locals, @@ -955,14 +966,14 @@ void HhbcTranslator::emitContExit() { void HhbcTranslator::emitUnpackCont() { gen(LinkContVarEnv, m_tb->getFp()); gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); - auto const cont = m_tb->genLdLoc(0); + auto const cont = ldLoc(0); push(gen(LdRaw, Type::Int, cont, cns(RawMemSlot::ContLabel))); } void HhbcTranslator::emitPackCont(int64_t labelId) { gen(UnlinkContVarEnv, m_tb->getFp()); gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); - auto const cont = m_tb->genLdLoc(0); + auto const cont = ldLoc(0); auto const newVal = popC(); auto const oldValue = gen(LdProp, Type::Cell, cont, cns(CONTOFF(m_value))); gen(StProp, cont, cns(CONTOFF(m_value)), newVal); @@ -974,7 +985,7 @@ void HhbcTranslator::emitPackCont(int64_t labelId) { void HhbcTranslator::emitContReceive() { gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); - auto const cont = m_tb->genLdLoc(0); + auto const cont = ldLoc(0); gen(ContRaiseCheck, getExitSlowTrace(), cont); auto const valOffset = cns(CONTOFF(m_received)); push(gen(LdProp, Type::Cell, cont, valOffset)); @@ -983,7 +994,7 @@ void HhbcTranslator::emitContReceive() { void HhbcTranslator::emitContRetC() { gen(AssertLoc, Type::Obj, LocalId(0), m_tb->getFp()); - auto const cont = m_tb->genLdLoc(0); + auto const cont = ldLoc(0); gen(ExitWhenSurprised, getExitSlowTrace()); gen( StRaw, cont, cns(RawMemSlot::ContDone), cns(true) @@ -1014,7 +1025,7 @@ void HhbcTranslator::emitContSendImpl(bool raise) { gen(ContPreNext, getExitSlowTrace(), cont); gen(AssertLoc, Type::Cell, LocalId(0), m_tb->getFp()); - auto const newVal = gen(IncRef, m_tb->genLdLoc(0)); + auto const newVal = gen(IncRef, ldLoc(0)); auto const oldVal = gen(LdProp, Type::Cell, cont, cns(CONTOFF(m_received))); gen(StProp, cont, cns(CONTOFF(m_received)), newVal); gen(DecRef, oldVal); @@ -1175,16 +1186,9 @@ void HhbcTranslator::emitMInstr(const NormalizedInstruction& ni) { * Unboxes var if necessary when var is not uninit. */ void HhbcTranslator::emitIssetL(int32_t id) { - Type trackedType = m_tb->getLocalType(id); - // guards should ensure we have type info at this point - assert(trackedType != Type::None); - if (trackedType.subtypeOf(Type::Uninit)) { - push(cns(false)); - } else { - Trace* exitTrace = getExitTrace(); - SSATmp* ld = m_tb->genLdLocAsCell(id, exitTrace); - push(gen(IsNType, Type::Null, ld)); - } + auto const exitTrace = getExitTrace(); + auto const ld = ldLocInner(id, exitTrace); + push(gen(IsNType, Type::Null, ld)); } void HhbcTranslator::emitIssetG(const StringData* gblName) { @@ -1200,15 +1204,9 @@ void HhbcTranslator::emitIssetS(const StringData* propName) { } void HhbcTranslator::emitEmptyL(int32_t id) { - Type trackedType = m_tb->getLocalType(id); - assert(trackedType != Type::None); - if (trackedType == Type::Uninit) { - push(cns(true)); - } else { - Trace* exitTrace = getExitTrace(); - SSATmp* ld = m_tb->genLdLocAsCell(id, exitTrace); - push(gen(OpNot, gen(ConvCellToBool, ld))); - } + auto const exitTrace = getExitTrace(); + auto const ld = ldLocInner(id, exitTrace); + push(gen(OpNot, gen(ConvCellToBool, ld))); } void HhbcTranslator::emitEmptyG(const StringData* gblName) { @@ -1231,7 +1229,7 @@ void HhbcTranslator::emitIsTypeC(Type t) { void HhbcTranslator::emitIsTypeL(Type t, int id) { Trace* exitTrace = getExitTrace(); - push(gen(IsType, t, emitLdLocWarn(id, exitTrace))); + push(gen(IsType, t, ldLocInnerWarn(id, exitTrace))); } void HhbcTranslator::emitIsNullL(int id) { emitIsTypeL(Type::Null, id);} @@ -1857,7 +1855,7 @@ void HhbcTranslator::emitFCallBuiltin(uint32_t numArgs, break; case KindOfDouble: assert(false); default: - args[i + 1] = loadStackAddr(numArgs - i - 1); + args[i + 1] = ldStackAddr(numArgs - i - 1); break; } } @@ -1957,8 +1955,21 @@ SSATmp* HhbcTranslator::emitDecRefLocalsInline(SSATmp* retVal) { for (int id = getCurFunc()->numLocals() - 1; id >= 0; --id) { if (retValLocId == id) { gen(DecRef, retVal); + continue; + } + + // When a parameter goes out of scope during return, we need to + // null it out so that debug_backtrace can't capture stale values. + // TODO(#2332775): we shouldn't have to do this. + const bool needSetNull = id < getCurFunc()->numParams(); + if (needSetNull) { + auto const loc = ldLoc(id); + if (needSetNull) { + gen(StLoc, LocalId(id), m_tb->getFp(), m_tb->genDefUninit()); + } + gen(DecRef, loc); } else { - m_tb->genDecRefLoc(id); + gen(DecRefLoc, Type::Gen, LocalId(id), m_tb->getFp()); } } @@ -2216,7 +2227,7 @@ void HhbcTranslator::emitLoadDeps() { for (auto& guard : m_typeGuards) { switch (guard.getKind()) { case TypeGuard::Local: - m_tb->genLdLoc(guard.getIndex()); + ldLoc(guard.getIndex()); break; case TypeGuard::Stack: break; @@ -2270,7 +2281,7 @@ Trace* HhbcTranslator::guardRefs(int64_t entryArDelta, void HhbcTranslator::emitVerifyParamType(int32_t paramId) { const Func* func = getCurFunc(); const TypeConstraint& tc = func->params()[paramId].typeConstraint(); - SSATmp* locVal = m_tb->genLdLoc(paramId); + auto locVal = ldLoc(paramId); Type locType = locVal->type().unbox(); assert(locType.isKnownDataType()); @@ -2580,7 +2591,7 @@ void HhbcTranslator::emitAGetC(const StringData* clsName) { } void HhbcTranslator::emitAGetL(int id, const StringData* clsName) { - SSATmp* src = m_tb->genLdLocAsCell(id, getExitTrace()); + auto const src = ldLocInner(id, getExitTrace()); if (isSupportedAGet(src, clsName)) { emitAGet(src, clsName); } else { @@ -3035,7 +3046,7 @@ void HhbcTranslator::exceptionBarrier() { gen(ExceptionBarrier, sp); } -SSATmp* HhbcTranslator::loadStackAddr(int32_t offset) { +SSATmp* HhbcTranslator::ldStackAddr(int32_t offset) { // You're almost certainly doing it wrong if you want to get the address of a // stack cell that's in m_evalStack. assert(offset >= (int32_t)m_evalStack.numCells()); @@ -3047,13 +3058,47 @@ SSATmp* HhbcTranslator::loadStackAddr(int32_t offset) { ); } -// -// This is a wrapper to TraceBuilder::genLdLoc() that also emits the -// RaiseUninitLoc if the local is uninitialized -// -SSATmp* HhbcTranslator::emitLdLocWarn(uint32_t id, - Trace* target) { - SSATmp* locVal = m_tb->genLdLocAsCell(id, target); +SSATmp* HhbcTranslator::ldLoc(uint32_t locId) { + return gen( + LdLoc, + Type::Gen, + LocalId(locId), + m_tb->getFp() + ); +} + +SSATmp* HhbcTranslator::ldLocAddr(uint32_t locId) { + return gen( + LdLocAddr, + Type::PtrToGen, + LocalId(locId), + m_tb->getFp() + ); +} + +/* + * Load a local, and if it's boxed dereference to get the inner cell. + * + * Note: For boxed values, this will generate a LdRef instruction which + * takes the given exit trace in case the inner type doesn't match + * the tracked type for this local. This check may be optimized away + * if we can determine that the inner type must match the tracked type. + */ +SSATmp* HhbcTranslator::ldLocInner(uint32_t locId, Trace* exitTrace) { + auto loc = ldLoc(locId); + assert((loc->type().isBoxed() || loc->type().notBoxed()) && + "Currently we don't handle traces where locals are maybeBoxed"); + return loc->type().isBoxed() + ? gen(LdRef, loc->type().innerType(), exitTrace, loc) + : loc; +} + +/* + * This is a wrapper to ldLocInner that also emits the RaiseUninitLoc + * if the local is uninitialized + */ +SSATmp* HhbcTranslator::ldLocInnerWarn(uint32_t id, Trace* target) { + auto const locVal = ldLocInner(id, target); if (locVal->type().subtypeOf(Type::Uninit)) { exceptionBarrier(); @@ -3064,6 +3109,59 @@ SSATmp* HhbcTranslator::emitLdLocWarn(uint32_t id, return locVal; } +/* + * Store to a local, if it's boxed set the value on the inner cell. + * + * Returns the value that was stored to the local, after incrementing + * its reference count. + * + * Pre: !newVal->type().isBoxed() && !newVal->type().maybeBoxed() + * Pre: exitTrace != nullptr if the local may be boxed + */ +SSATmp* HhbcTranslator::stLocImpl(uint32_t id, + Trace* exitTrace, + SSATmp* newVal, + bool doRefCount) { + assert(!newVal->type().maybeBoxed()); + + auto const oldLoc = ldLoc(id); + assert((oldLoc->type().isBoxed() || oldLoc->type().notBoxed()) && + "We don't support maybeBoxed locals right now"); + + if (oldLoc->type().notBoxed()) { + gen(StLoc, LocalId(id), m_tb->getFp(), newVal); + auto const ret = doRefCount ? gen(IncRef, newVal) : newVal; + if (doRefCount) { + gen(DecRef, oldLoc); + } + return ret; + } + + // It's important that the IncRef happens after the LdRef, since the + // LdRef is also a guard on the inner type and may side-exit. + assert(exitTrace); + auto const innerCell = gen( + LdRef, oldLoc->type().innerType(), exitTrace, oldLoc + ); + auto const ret = doRefCount ? gen(IncRef, newVal) : newVal; + gen(StRef, oldLoc, newVal); + if (doRefCount) { + gen(DecRef, innerCell); + } + + return ret; +} + +SSATmp* HhbcTranslator::stLoc(uint32_t id, Trace* exit, SSATmp* newVal) { + const bool doRefCount = true; + return stLocImpl(id, exit, newVal, doRefCount); +} + +SSATmp* HhbcTranslator::stLocNRC(uint32_t id, Trace* exit, SSATmp* newVal) { + const bool doRefCount = false; + return stLocImpl(id, exit, newVal, doRefCount); +} + void HhbcTranslator::end(int nextPc) { if (m_hasExit) return; diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.h b/hphp/runtime/vm/translator/hopt/hhbctranslator.h index 6d9a3ea69..0ce248bc2 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.h +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.h @@ -547,7 +547,6 @@ private: Trace* getExitTrace(uint32_t targetBcOff, uint32_t notTakenBcOff); Trace* getExitSlowTrace(); Trace* getGuardExit(); - SSATmp* emitLdLocWarn(uint32_t id, Trace* target); void emitInterpOneOrPunt(Type type, int numPopped, int numExtraPushed = 0); void emitBinaryArith(Opcode); template @@ -580,7 +579,7 @@ private: const NamedEntityPair& lookupNamedEntityPairId(int id); /* - * Eval stack helpers + * Eval stack helpers. */ SSATmp* push(SSATmp* tmp); SSATmp* pushIncRef(SSATmp* tmp) { return push(gen(IncRef, tmp)); } @@ -596,12 +595,23 @@ private: std::vector getSpillValues() const; SSATmp* spillStack(); void exceptionBarrier(); - SSATmp* loadStackAddr(int32_t offset); + SSATmp* ldStackAddr(int32_t offset); SSATmp* top(Type type, uint32_t index = 0); void extendStack(uint32_t index, Type type); void replace(uint32_t index, SSATmp* tmp); void refineType(SSATmp* tmp, Type type); + /* + * Local instruction helpers. + */ + SSATmp* ldLoc(uint32_t id); + SSATmp* ldLocAddr(uint32_t id); + SSATmp* ldLocInner(uint32_t id, Trace* exitTrace); + SSATmp* ldLocInnerWarn(uint32_t id, Trace* target); + SSATmp* stLoc(uint32_t id, Trace* exitTrace, SSATmp* newVal); + SSATmp* stLocNRC(uint32_t id, Trace* exitTrace, SSATmp* newVal); + SSATmp* stLocImpl(uint32_t id, Trace*, SSATmp* newVal, bool doRefCount); + private: // Tracks information about the current bytecode offset and which // function we are in. Goes in m_bcStateStack; we push and pop as diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index e45ded4d7..fb0e0d4be 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -343,8 +343,8 @@ O(StProp, ND, S(Obj) S(Int) S(Gen), E|Mem|CRc|Refs) \ O(StPropNT, ND, S(Obj) S(Int) S(Gen), E|Mem|CRc) \ O(StLoc, ND, S(FramePtr) S(Gen), E|Mem|CRc) \ O(StLocNT, ND, S(FramePtr) S(Gen), E|Mem|CRc) \ -O(StRef, DBox(1), SUnk, E|Mem|CRc|Refs) \ -O(StRefNT, DBox(1), SUnk, E|Mem|CRc) \ +O(StRef, DBox(1), S(BoxedCell) S(Cell), E|Mem|CRc|Refs) \ +O(StRefNT, DBox(1), S(BoxedCell) S(Cell), E|Mem|CRc) \ O(StRaw, ND, SUnk, E|Mem) \ O(LdStaticLocCached, D(BoxedCell), C(CacheHandle), NF) \ O(StaticLocInit, D(BoxedCell), CStr \ diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index 7540015b2..85086e6d0 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -295,6 +295,9 @@ SSATmp* Simplifier::simplify(IRInstruction* inst) { case LdStack: return simplifyLdStack(inst); case LdStackAddr: return simplifyLdStackAddr(inst); case DecRefStack: return simplifyDecRefStack(inst); + case DecRefLoc: return simplifyDecRefLoc(inst); + case LdLoc: return simplifyLdLoc(inst); + case StRef: return simplifyStRef(inst); case ExitOnVarEnv: return simplifyExitOnVarEnv(inst); @@ -1615,6 +1618,32 @@ SSATmp* Simplifier::simplifyLdStack(IRInstruction* inst) { return nullptr; } +SSATmp* Simplifier::simplifyDecRefLoc(IRInstruction* inst) { + if (inst->getTypeParam().notCounted()) { + inst->convertToNop(); + } + return nullptr; +} + +SSATmp* Simplifier::simplifyLdLoc(IRInstruction* inst) { + if (inst->getTypeParam().isNull()) { + return cns(inst->getTypeParam()); + } + return nullptr; +} + +// Replace StRef with StRefNT when we know we aren't going to change +// its m_type field. +SSATmp* Simplifier::simplifyStRef(IRInstruction* inst) { + auto const oldUnbox = inst->getSrc(0)->type().unbox(); + auto const newType = inst->getSrc(1)->type(); + if (oldUnbox.isKnownDataType() && + oldUnbox.equals(newType) && !oldUnbox.isString()) { + inst->setOpcode(StRefNT); + } + return nullptr; +} + SSATmp* Simplifier::simplifyLdStackAddr(IRInstruction* inst) { auto const info = getStackValue(inst->getSrc(0), inst->getExtra()->offset); diff --git a/hphp/runtime/vm/translator/hopt/simplifier.h b/hphp/runtime/vm/translator/hopt/simplifier.h index 3688c76de..fabac36de 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.h +++ b/hphp/runtime/vm/translator/hopt/simplifier.h @@ -123,6 +123,9 @@ private: SSATmp* simplifyLdStack(IRInstruction*); SSATmp* simplifyLdStackAddr(IRInstruction*); SSATmp* simplifyDecRefStack(IRInstruction*); + SSATmp* simplifyDecRefLoc(IRInstruction*); + SSATmp* simplifyLdLoc(IRInstruction*); + SSATmp* simplifyStRef(IRInstruction*); private: // tracebuilder forwarders template SSATmp* cns(Args&&...); diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index 3c2036aaf..47359bc13 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -28,10 +28,6 @@ namespace JIT { static const HPHP::Trace::Module TRACEMOD = HPHP::Trace::hhir; -static inline Type noneToGen(Type t) { - return t.subtypeOf(Type::None) ? Type::Gen : t; -} - TraceBuilder::TraceBuilder(Offset initialBcOffset, uint32_t initialSpOffsetFromFp, IRFactory& irFactory, @@ -226,190 +222,6 @@ SSATmp* TraceBuilder::genDefNone() { return gen(DefConst, Type::None, ConstData(0)); } -SSATmp* TraceBuilder::genBoxLoc(uint32_t id) { - SSATmp* prevValue = genLdLoc(id); - Type prevType = prevValue->type(); - // Don't box if local's value already boxed - if (prevType.isBoxed()) { - return prevValue; - } - assert(prevType.notBoxed()); - // The Box helper requires us to incref the values its boxing, but in - // this case we don't need to incref prevValue because we are simply - // transfering its refcount from the local to the box. - if (prevValue->isA(Type::Uninit)) { - // No box can ever contain Uninit, so promote it to InitNull here. - prevValue = genDefInitNull(); - } - SSATmp* newValue = gen(Box, prevValue); - gen(StLoc, LocalId(id), m_fpValue, newValue); - return newValue; -} - -/** - * Returns an SSATmp containing the current value of the given local. - * This generates a LdLoc instruction if needed. - * - * Note: the type of the local must be known already (due to type guards - * or assertions). - */ -SSATmp* TraceBuilder::genLdLoc(uint32_t id) { - SSATmp* tmp = getLocalValue(id); - if (tmp) { - return tmp; - } - // No prior value for this local is available, so actually generate a LdLoc. - auto type = getLocalType(id); - assert(type != Type::None); // tracelet guards guarantee we have a type - assert(type != Type::Null); // we can get Uninit or InitNull but not both - if (type.isNull()) { - tmp = cns(type); - } else { - tmp = gen(LdLoc, type, LocalId(id), m_fpValue); - } - return tmp; -} - -SSATmp* TraceBuilder::genLdLocAsCell(uint32_t id, Trace* exitTrace) { - SSATmp* tmp = genLdLoc(id); - Type type = tmp->type(); - assert(type.isBoxed() || type.notBoxed()); - if (!type.isBoxed()) { - return tmp; - } - // Unbox tmp into a cell via a LdRef - return gen(LdRef, type.innerType(), exitTrace, tmp); -} - -SSATmp* TraceBuilder::genLdLocAddr(uint32_t id) { - return gen(LdLocAddr, getLocalType(id).ptr(), LocalId(id), getFp()); -} - -void TraceBuilder::genDecRefLoc(int id) { - Type type = getLocalType(id); - - // Don't generate code if type is not refcounted - if (type != Type::None && type.notCounted()) { - return; - } - type = noneToGen(type); - // When a parameter goes out of scope, we need to null - // it out so that debug_backtrace can't capture stale - // values. - bool setNull = id < m_curFunc->getValFunc()->numParams(); - SSATmp* val = getLocalValue(id); - if (val == nullptr && setNull) { - val = gen(LdLoc, type, LocalId(id), m_fpValue); - } - if (val) { - if (setNull) { - gen(StLoc, LocalId(id), m_fpValue, genDefUninit()); - } - gen(DecRef, val); - return; - } - - if (type.isBoxed()) { - // we can't really rely on the types held in the boxed values since - // aliasing stores may change them. We conservatively set the type - // of the decref to a boxed cell. - type = Type::BoxedCell; - } - - gen(DecRefLoc, type, LocalId(id), m_fpValue); -} - -/* - * Stores a ref (boxed value) to a local. Also handles unsetting a local. - */ -void TraceBuilder::genBindLoc(uint32_t id, - SSATmp* newValue, - bool doRefCount /* = true */) { - Type trackedType = getLocalType(id); - SSATmp* prevValue = 0; - if (trackedType == Type::None) { - if (doRefCount) { - prevValue = gen(LdLoc, Type::Gen, LocalId(id), m_fpValue); - } - } else { - prevValue = getLocalValue(id); - assert(prevValue == nullptr || prevValue->type() == trackedType); - if (prevValue == newValue) { - // Silent store: local already contains value being stored - // NewValue needs to be decref'ed - if (!trackedType.notCounted() && doRefCount) { - gen(DecRef, prevValue); - } - return; - } - if (trackedType.maybeCounted() && !prevValue && doRefCount) { - prevValue = gen(LdLoc, trackedType, LocalId(id), m_fpValue); - } - } - bool genStoreType = true; - if ((trackedType.isBoxed() && newValue->type().isBoxed()) || - (trackedType == newValue->type() && !trackedType.isString())) { - // no need to store type with local value - genStoreType = false; - } - gen(genStoreType ? StLoc : StLocNT, LocalId(id), m_fpValue, newValue); - if (trackedType.maybeCounted() && doRefCount) { - gen(DecRef, prevValue); - } -} - -/* - * Store a cell value to a local that might be boxed. - */ -SSATmp* TraceBuilder::genStLoc(uint32_t id, - SSATmp* newValue, - bool doRefCount, - bool genStoreType, - Trace* exit) { - assert(!newValue->type().isBoxed()); - /* - * If prior value of local is a cell, then re-use genBindLoc. - * Otherwise, if prior value of local is a ref: - * - * prevLocValue = LdLoc{id} fp - * prevValue = LdRef [prevLocValue] - * newRef = StRef [prevLocValue], newValue - * DecRef prevValue - * -- track local value in newRef - */ - Type trackedType = getLocalType(id); - assert(trackedType != Type::None); // tracelet guards guarantee a type - if (trackedType.notBoxed()) { - SSATmp* retVal = doRefCount ? gen(IncRef, newValue) : newValue; - genBindLoc(id, newValue, doRefCount); - return retVal; - } - assert(trackedType.isBoxed()); - SSATmp* prevRef = getLocalValue(id); - assert(prevRef == nullptr || prevRef->type() == trackedType); - // prevRef is a ref - if (prevRef == nullptr) { - // prevRef = ldLoc - prevRef = gen(LdLoc, trackedType, LocalId(id), m_fpValue); - } - SSATmp* prevValue = nullptr; - if (doRefCount) { - assert(exit); - Type innerType = trackedType.innerType(); - prevValue = gen(LdRef, innerType, exit, prevRef); - } - // stref [prevRef] = t1 - Opcode opc = genStoreType ? StRef : StRefNT; - gen(opc, prevRef, newValue); - - SSATmp* retVal = newValue; - if (doRefCount) { - retVal = gen(IncRef, newValue); - gen(DecRef, prevValue); - } - return retVal; -} - void TraceBuilder::updateTrackedState(IRInstruction* inst) { Opcode opc = inst->op(); // Update tracked state of local values/types, stack/frame pointer, CSE, etc. @@ -930,6 +742,81 @@ SSATmp* TraceBuilder::preOptimizeDecRefThis(IRInstruction* inst) { return nullptr; } +SSATmp* TraceBuilder::preOptimizeDecRefLoc(IRInstruction* inst) { + auto const locId = inst->getExtra()->locId; + + /* + * Refine the type if we can. + * + * We can't really rely on the types held in the boxed values since + * aliasing stores may change them, and we only guard during LdRef. + * So we have to change any boxed type to BoxedCell. + */ + auto knownType = getLocalType(locId); + if (knownType.isBoxed()) { + knownType = Type::BoxedCell; + } + if (knownType != Type::None) { // TODO(#2135185) + inst->setTypeParam( + Type::mostRefined(knownType, inst->getTypeParam()) + ); + } + + /* + * If we have the local value in flight, use a DecRef on it instead + * of doing it in memory. + */ + if (auto tmp = getLocalValue(locId)) { + gen(DecRef, tmp); + inst->convertToNop(); + } + + return nullptr; +} + +SSATmp* TraceBuilder::preOptimizeLdLoc(IRInstruction* inst) { + auto const locId = inst->getExtra()->locId; + if (auto tmp = getLocalValue(locId)) { + return tmp; + } + if (getLocalType(locId) != Type::None) { // TODO(#2135185) + inst->setTypeParam( + Type::mostRefined(getLocalType(locId), inst->getTypeParam()) + ); + } + return nullptr; +} + +SSATmp* TraceBuilder::preOptimizeLdLocAddr(IRInstruction* inst) { + auto const locId = inst->getExtra()->locId; + if (getLocalType(locId) != Type::None) { // TODO(#2135185) + inst->setTypeParam( + Type::mostRefined(getLocalType(locId).ptr(), inst->getTypeParam()) + ); + } + return nullptr; +} + +SSATmp* TraceBuilder::preOptimizeStLoc(IRInstruction* inst) { + auto const curType = getLocalType(inst->getExtra()->locId); + auto const newType = inst->getSrc(1)->type(); + + assert(inst->getTypeParam().equals(Type::None)); + + // There's no need to store the type if it's going to be the same + // KindOfFoo. We still have to store string types because we don't + // guard on KindOfStaticString vs. KindOfString. + auto const bothBoxed = curType.isBoxed() && newType.isBoxed(); + auto const sameUnboxed = curType != Type::None && // TODO(#2135185) + curType.isKnownDataType() && + curType.equals(newType) && !curType.isString(); + if (bothBoxed || sameUnboxed) { + inst->setOpcode(StLocNT); + } + + return nullptr; +} + SSATmp* TraceBuilder::preOptimize(IRInstruction* inst) { #define X(op) case op: return preOptimize##op(inst) switch (inst->op()) { @@ -939,6 +826,10 @@ SSATmp* TraceBuilder::preOptimize(IRInstruction* inst) { X(LdCtx); X(DecRef); X(DecRefThis); + X(DecRefLoc); + X(LdLoc); + X(LdLocAddr); + X(StLoc); default: break; } @@ -1186,7 +1077,7 @@ void TraceBuilder::dropLocalRefsInnerTypes() { * locals, however. */ void TraceBuilder::killLocals() { - for (uint32_t i = 0; i < m_localValues.size(); i++) { + for (uint32_t i = 0; i < m_localValues.size(); i++) { SSATmp* t = m_localValues[i]; // should not kill DefConst, and LdConst should be replaced by DefConst if (!t || t->inst()->op() == DefConst) { diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.h b/hphp/runtime/vm/translator/hopt/tracebuilder.h index b6878e19c..d53d126c9 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.h +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.h @@ -101,6 +101,7 @@ struct TraceBuilder { int32_t prevSPOff); void endInlining(); + // XXX: Accessible for inlining, for now. void setLocalValue(unsigned id, SSATmp* value); void setEnableCse(bool val) { m_enableCse = val; } @@ -153,36 +154,6 @@ struct TraceBuilder { return instr; } - ////////////////////////////////////////////////////////////////////// - // locals - - Type getLocalType(unsigned id) const; - - SSATmp* genLdLoc(uint32_t id); - SSATmp* genLdLocAddr(uint32_t id); - - /* - * Returns an SSATmp containing the (inner) value of the given local. - * If the local is a boxed value, this returns its inner value. - * - * Note: For boxed values, this will generate a LdRef instruction which - * takes the given exit trace in case the inner type doesn't match - * the tracked type for this local. This check may be optimized away - * if we can determine that the inner type must match the tracked type. - */ - SSATmp* genLdLocAsCell(uint32_t id, Trace* exitTrace); - - SSATmp* genStLoc(uint32_t id, - SSATmp* src, - bool doRefCount, - bool genStoreType, - Trace* exit); - - SSATmp* genBoxLoc(uint32_t id); - void genBindLoc(uint32_t id, SSATmp* ref, bool doRefCount = true); - - void genDecRefLoc(int id); - ////////////////////////////////////////////////////////////////////// // constants @@ -368,6 +339,10 @@ private: SSATmp* preOptimizeLdCtx(IRInstruction*); SSATmp* preOptimizeDecRef(IRInstruction*); SSATmp* preOptimizeDecRefThis(IRInstruction*); + SSATmp* preOptimizeDecRefLoc(IRInstruction*); + SSATmp* preOptimizeLdLoc(IRInstruction*); + SSATmp* preOptimizeLdLocAddr(IRInstruction*); + SSATmp* preOptimizeStLoc(IRInstruction*); SSATmp* preOptimize(IRInstruction* inst); SSATmp* optimizeWork(IRInstruction* inst); @@ -386,6 +361,7 @@ private: void killLocals(); void killLocalValue(uint32_t id); void setLocalType(uint32_t id, Type type); + Type getLocalType(unsigned id) const; SSATmp* getLocalValue(unsigned id) const; bool isValueAvailable(SSATmp*) const; bool anyLocalHasValue(SSATmp*) const; diff --git a/hphp/runtime/vm/translator/hopt/vectortranslator.cpp b/hphp/runtime/vm/translator/hopt/vectortranslator.cpp index 1af39c11b..f682eab34 100644 --- a/hphp/runtime/vm/translator/hopt/vectortranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/vectortranslator.cpp @@ -515,7 +515,7 @@ SSATmp* HhbcTranslator::VectorTranslator::getInput(unsigned i) { } case Location::Local: - return m_tb.genLdLoc(l.offset); + return m_ht.ldLoc(l.offset); case Location::Litstr: return cns(m_ht.lookupStringId(l.offset)); @@ -544,8 +544,12 @@ void HhbcTranslator::VectorTranslator::emitBaseLCR() { gen(RaiseUninitLoc, LocalId(base.location.offset)); } if (mia & MIA_define) { - m_tb.genStLoc(base.location.offset, m_tb.genDefInitNull(), - false, true, nullptr); + gen( + StLoc, + LocalId(base.location.offset), + m_tb.getFp(), + m_tb.genDefInitNull() + ); baseType = Type::InitNull; } } @@ -575,12 +579,12 @@ void HhbcTranslator::VectorTranslator::emitBaseLCR() { } if (base.location.space == Location::Local) { - m_base = m_tb.genLdLocAddr(base.location.offset); + m_base = m_ht.ldLocAddr(base.location.offset); } else { assert(base.location.space == Location::Stack); m_ht.spillStack(); assert(m_stackInputs.count(m_iInd)); - m_base = m_ht.loadStackAddr(m_stackInputs[m_iInd]); + m_base = m_ht.ldStackAddr(m_stackInputs[m_iInd]); } assert(m_base->type().isPtr()); } @@ -1711,8 +1715,8 @@ static inline ArrayData* checkedSet(ArrayData* a, int64_t key, template static inline typename ShuffleReturn::return_type arraySetImpl( - ArrayData* a, typename KeyTypeTraits::rawType key, - CVarRef value, RefData* ref) { + ArrayData* a, typename KeyTypeTraits::rawType key, + CVarRef value, RefData* ref) { static_assert(keyType != AnyKey, "AnyKey is not supported in arraySetMImpl"); const bool copy = a->getCount() > 1; ArrayData* ret = checkForInt ? checkedSet(a, key, value, copy) @@ -1772,7 +1776,9 @@ void HhbcTranslator::VectorTranslator::emitArraySet(SSATmp* key, // Update the base's value with the new array if (base.location.space == Location::Local) { - m_tb.genStLoc(base.location.offset, newArr, false, false, nullptr); + // We know it's not boxed (setRef above handles that), and + // newArr has already been incref'd in the helper. + gen(StLoc, LocalId(base.location.offset), m_tb.getFp(), newArr); } else if (base.location.space == Location::Stack) { VectorEffects ve(newArr->inst()); assert(ve.baseValChanged);