diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index d1d35942c..57d69c266 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -297,6 +297,18 @@ KillsSource The instruction might destroy one or more of its sources, so they cannot be safely used after it has executed. +HasStackVersion + + This instruction has a counterpart that returns a StkPtr in addition to any + primary destination. The behavior of the stack-modifying version is otherwise + identical. + +ModifiesStack + + The instruction modifies the in-memory evaluation stack in the process of + performing its primary work. It will have a StkPtr destination in addition to + its primary destination. + Instruction set --------------- @@ -1150,6 +1162,7 @@ D:Cell = CGetProp S0:ConstTCA S1:ConstCls S2:{Obj|PtrToGen} S3:Gen S4:PtrToCell struct. D:Vector = SetProp S0:ConstTCA S1:ConstCls S2:{Obj|PtrToGen} S3:Gen S4:Cell +D:Vector, D:StkPtr = SetPropStk S0:ConstTCA S1:ConstCls S2:{Obj|PtrToGen} S3:Gen S4:Cell Set property with key S3 in S2 to S4. @@ -1159,6 +1172,7 @@ D:PtrToGen = ElemX S0:ConstTCA S1:PtrToGen S2:Gen S3:PtrToCell modified. S3 should point to an MInstrState struct. D:PtrToGen = ElemDX S0:ConstTCA S1:PtrToGen S2:Gen S3:PtrToCell +D:PtrToGen, D:StkPtr = ElemDXStk S0:ConstTCA S1:PtrToGen S2:Gen S3:PtrToCell Like ElemX, but used for intermediate element lookups that may modify the base. @@ -1180,10 +1194,12 @@ ArraySet S0:ConstTCA S1:Arr S2:{Int|Str} S3:Cell S4:BoxedArr operation. D:Vector = SetElem S0:ConstTCA S1:PtrToGen S2:Gen S3:Cell +D:Vector, D:StkPtr = SetElemStk S0:ConstTCA S1:PtrToGen S2:Gen S3:Cell Set element with key S2 in S1 to S3. -D:Vector = SetNewElem S0:GenPtr S1:Cell +D:Vector = SetNewElem S0:PtrToGen S1:Cell +D:Vector, D:StkPtr = SetNewElemStk S0:PtrToGen S1:Cell Append the value in S1 to S0. diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 5c309a5b9..ce1521024 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -307,6 +307,10 @@ Address CodeGenerator::cgInst(IRInstruction* inst) { #define CALL_OPCODE(opcode) \ void CodeGenerator::cg##opcode(IRInstruction* i) { cgCallNative(i); } +#define CALL_STK_OPCODE(opcode) \ + CALL_OPCODE(opcode) \ + CALL_OPCODE(opcode ## Stk) + NOOP_OPCODE(DefConst) NOOP_OPCODE(DefFP) NOOP_OPCODE(DefSP) @@ -342,14 +346,14 @@ CALL_OPCODE(RaiseUndefProp) CALL_OPCODE(BaseG) CALL_OPCODE(PropX) CALL_OPCODE(CGetProp) -CALL_OPCODE(SetProp) +CALL_STK_OPCODE(SetProp) CALL_OPCODE(ElemX) -CALL_OPCODE(ElemDX) +CALL_STK_OPCODE(ElemDX) CALL_OPCODE(CGetElem) CALL_OPCODE(ArraySet) CALL_OPCODE(ArraySetRef) -CALL_OPCODE(SetElem) -CALL_OPCODE(SetNewElem) +CALL_STK_OPCODE(SetElem) +CALL_STK_OPCODE(SetNewElem) CALL_OPCODE(IssetElem) CALL_OPCODE(EmptyElem) @@ -707,7 +711,7 @@ void CodeGenerator::cgCallNative(IRInstruction* inst) { } cgCallHelper(m_as, addr, - info.dest != DestType::None ? inst->getDst() : nullptr, + info.dest != DestType::None ? inst->getDst(0) : nullptr, info.sync, argGroup, info.dest); diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index afac1b649..b71a83581 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -2516,7 +2516,8 @@ Trace* HhbcTranslator::getExitTrace(Offset targetBcOff /* = -1 */) { * Generates a trace exit that can be the target of a conditional * control flow instruction at the current bytecode offset. */ -Trace* HhbcTranslator::getExitTrace(uint32_t targetBcOff, uint32_t notTakenBcOff) { +Trace* HhbcTranslator::getExitTrace(uint32_t targetBcOff, + uint32_t notTakenBcOff) { std::vector stackValues = getSpillValues(); return m_tb->genExitTrace(targetBcOff, m_stackDeficit, diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.h b/hphp/runtime/vm/translator/hopt/hhbctranslator.h index f9b5a6d9a..f3fbec8f7 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.h +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.h @@ -441,6 +441,15 @@ private: SSATmp* checkInitProp(SSATmp* baseAsObj, SSATmp* propAddr, int propOffset, bool warn, bool define); + /* + * genStk is a wrapper around TraceBuilder::gen() to deal with instructions + * that may modify the stack. It inspects the opcode and the types of the + * inputs, replacing the opcode with the version that returns a new StkPtr + * if appropriate. + */ + template + SSATmp* genStk(Opcode op, Srcs... srcs); + bool isSimpleArraySet(); bool generateMVal() const; bool needFirstRatchet() const; diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index 89565e528..489cc0c0f 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -79,25 +79,6 @@ TRACE_SET_MOD(hhir); namespace { -enum OpcodeFlag : uint64_t { - NoFlags = 0x0000, - HasDest = 0x0001, - CanCSE = 0x0002, - Essential = 0x0004, - MemEffects = 0x0008, - CallsNative = 0x0010, - ConsumesRC = 0x0020, - ProducesRC = 0x0040, - MayModifyRefs = 0x0080, - Rematerializable = 0x0100, // TODO: implies HasDest - MayRaiseError = 0x0200, - Terminal = 0x0400, // has no next instruction - NaryDest = 0x0800, // has 0 or more destinations - HasExtra = 0x1000, - Passthrough = 0x2000, - KillsSources = 0x4000, -}; - #define NF 0 #define C CanCSE #define E Essential @@ -111,6 +92,7 @@ enum OpcodeFlag : uint64_t { #define T Terminal #define P Passthrough #define K KillsSources +#define StkFlags(f) HasStackVersion|(f) #define ND 0 #define D(n) HasDest @@ -118,8 +100,9 @@ enum OpcodeFlag : uint64_t { #define DUnbox(n) HasDest #define DBox(n) HasDest #define DParam HasDest -#define DLabel NaryDest +#define DMulti NaryDest #define DVector HasDest +#define DStk(x) ModifiesStack|(x) #define DPtrToParam HasDest #define DBuiltin HasDest @@ -149,6 +132,7 @@ struct { #undef T #undef P #undef K +#undef StkFlags #undef ND #undef D @@ -156,7 +140,10 @@ struct { #undef DUnbox #undef DBox #undef DParam -#undef DLabel +#undef DMulti +#undef DVector +#undef DStk +#undef DPtrToParam #undef DBuiltin /* @@ -309,20 +296,31 @@ IRInstruction::IRInstruction(Arena& arena, const IRInstruction* inst, IId iid) const char* opcodeName(Opcode opcode) { return OpInfo[opcode].name; } -static bool opcodeHasFlags(Opcode opcode, uint64_t flags) { +bool opcodeHasFlags(Opcode opcode, uint64_t flags) { return OpInfo[opcode].flags & flags; } +Opcode getStackModifyingOpcode(Opcode opc) { + assert(opcodeHasFlags(opc, HasStackVersion)); + opc = Opcode(opc + 1); + assert(opcodeHasFlags(opc, ModifiesStack)); + return opc; +} + bool IRInstruction::hasExtra() const { return opcodeHasFlags(getOpcode(), HasExtra) && m_extra; } +// Instructions with ModifiesStack are always naryDst regardless of +// the inner dest. + bool IRInstruction::hasDst() const { - return opcodeHasFlags(getOpcode(), HasDest); + return opcodeHasFlags(getOpcode(), HasDest) && + !opcodeHasFlags(getOpcode(), ModifiesStack); } bool IRInstruction::naryDst() const { - return opcodeHasFlags(getOpcode(), NaryDest); + return opcodeHasFlags(getOpcode(), NaryDest | ModifiesStack); } bool IRInstruction::isNative() const { @@ -460,6 +458,22 @@ bool IRInstruction::killsSource(int idx) const { not_reached(); } +bool IRInstruction::modifiesStack() const { + return opcodeHasFlags(getOpcode(), ModifiesStack); +} + +SSATmp* IRInstruction::modifiedStkPtr() const { + assert(modifiesStack()); + assert(VectorEffects::supported(this)); + SSATmp* sp = getDst(hasMainDst() ? 1 : 0); + assert(sp->isA(Type::StkPtr)); + return sp; +} + +bool IRInstruction::hasMainDst() const { + return opcodeHasFlags(getOpcode(), HasDest); +} + bool IRInstruction::mayReenterHelper() const { if (isCmpOp(getOpcode())) { return cmpOpTypesMayReenter(getOpcode(), @@ -472,8 +486,10 @@ bool IRInstruction::mayReenterHelper() const { } SSATmp* IRInstruction::getDst(unsigned i) const { - assert(naryDst() && i < m_numDsts); - return &m_dst[i]; + if (i == 0 && m_numDsts == 0) return nullptr; + assert(i < m_numDsts); + assert(naryDst() || i == 0); + return hasDst() ? getDst() : &m_dst[i]; } DstRange IRInstruction::getDsts() { @@ -679,13 +695,15 @@ void IRInstruction::printOpcode(std::ostream& os) const { } void IRInstruction::printDst(std::ostream& ostream) const { - if (unsigned n = m_numDsts) { - for (unsigned i = 0; i < n; ++i) { - if (i > 0) ostream << ", "; - m_dst->print(ostream, true); - } - ostream << " = "; + if (getNumDsts() == 0) return; + + const char* sep = ""; + for (const SSATmp& dst : getDsts()) { + ostream << sep; + dst.print(ostream, true); + sep = ", "; } + ostream << " = "; } void IRInstruction::printSrc(std::ostream& ostream, uint32_t i) const { @@ -739,8 +757,7 @@ void IRInstruction::print(std::ostream& ostream) const { printSrc(ostream, 0); SSATmp* offset = getSrc(1); if (m_op == StRaw) { - RawMemSlot& s = - RawMemSlot::Get(RawMemSlot::Kind(offset->getValInt())); + RawMemSlot& s = RawMemSlot::Get(RawMemSlot::Kind(offset->getValInt())); int64_t offset = s.getOffset(); if (offset) { ostream << " + " << offset; diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 7925f4bcd..a7e5b5153 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -98,8 +98,12 @@ static const TCA kIRDirectGuardActive = (TCA)0x03; * DUnbox(N) single dst has unboxed type of src N * DBox(N) single dst has boxed type of src N * DParam single dst has type of the instruction's type parameter - * DLabel multiple dests for a DefLabel + * DMulti multiple dests. type and number depend on instruction * DVector single dst depends on semantics of the vector instruction + * DStk(x) up to two dests. x should be another D* macro and indicates + * the type of the first dest, if any. the second (or first, + * depending on the presence of a primary destination), will be + * of type Type::StkPtr. implies ModifiesStack. * DBuiltin single dst for CallBuiltin. This can return complex data * types such as (Type::Str | Type::Null) * @@ -140,6 +144,11 @@ static const TCA kIRDirectGuardActive = (TCA)0x03; * P passthrough * K killsSource */ + +#define O_STK(name, dst, src, flags) \ + O(name, dst, src, StkFlags(flags)) \ + O(name ## Stk, DStk(dst), src, flags) + #define IR_OPCODES \ /* name dstinfo srcinfo flags */ \ O(GuardType, DParam, S(Gen), C|E|CRc|PRc|P) \ @@ -208,7 +217,7 @@ O(UnboxPtr, D(PtrToCell), S(PtrToGen), NF) \ O(BoxPtr, D(PtrToBoxedCell), S(PtrToGen), N|Mem) \ O(LdStack, DParam, S(StkPtr) C(Int), NF) \ O(LdLoc, DParam, S(StkPtr), NF) \ -O(LdStackAddr, D(PtrToGen), SUnk, C) \ +O(LdStackAddr, DParam, S(StkPtr) C(Int), C) \ O(LdLocAddr, DParam, S(StkPtr), C) \ O(LdMem, DParam, S(PtrToGen) C(Int), NF) \ O(LdProp, DParam, S(Obj) C(Int), NF) \ @@ -300,7 +309,7 @@ O(DecRef, ND, S(Gen), N|E|Mem|CRc|Refs|K) \ O(DecRefMem, ND, S(PtrToGen) \ C(Int), N|E|Mem|CRc|Refs) \ O(DecRefNZ, ND, S(Gen), Mem|CRc) \ -O(DefLabel, DLabel, SUnk, E) \ +O(DefLabel, DMulti, SUnk, E) \ O(Marker, ND, NA, E) \ O(DefFP, D(StkPtr), NA, E) \ O(DefSP, D(StkPtr), S(StkPtr) C(Int), E) \ @@ -374,7 +383,7 @@ O(CGetProp, D(Cell), C(TCA) \ S(Obj,PtrToGen) \ S(Gen) \ S(PtrToCell), E|N|Mem|Refs|Er) \ -O(SetProp, DVector, C(TCA) \ +O_STK(SetProp, DVector, C(TCA) \ C(Cls) \ S(Obj,PtrToGen) \ S(Gen) \ @@ -383,7 +392,7 @@ O(ElemX, D(PtrToGen), C(TCA) \ S(PtrToGen) \ S(Gen) \ S(PtrToCell), E|N|Mem|Refs|Er) \ -O(ElemDX, D(PtrToGen), C(TCA) \ +O_STK(ElemDX, D(PtrToGen), C(TCA) \ S(PtrToGen) \ S(Gen) \ S(PtrToCell), E|N|Mem|Refs|Er) \ @@ -400,11 +409,11 @@ O(ArraySetRef, ND, C(TCA) \ S(Int,Str) \ S(Cell) \ S(BoxedArr),E|N|PRc|CRc|Refs|Mem|K) \ -O(SetElem, DVector, C(TCA) \ +O_STK(SetElem, DVector, C(TCA) \ S(PtrToGen) \ S(Gen) \ S(Cell), E|N|Mem|Refs|Er) \ -O(SetNewElem, DVector, S(PtrToGen) S(Gen), E|N|Mem|Refs|Er) \ +O_STK(SetNewElem, DVector, S(PtrToGen) S(Gen), E|N|Mem|Refs|Er) \ O(IssetElem, D(Bool), C(TCA) \ S(PtrToGen) \ S(Gen) \ @@ -747,6 +756,30 @@ inline bool isExitSlow(TraceExitType::ExitType t) { const char* opcodeName(Opcode opcode); +enum OpcodeFlag : uint64_t { + NoFlags = 0, + HasDest = 1ULL << 0, + CanCSE = 1ULL << 1, + Essential = 1ULL << 2, + MemEffects = 1ULL << 3, + CallsNative = 1ULL << 4, + ConsumesRC = 1ULL << 5, + ProducesRC = 1ULL << 6, + MayModifyRefs = 1ULL << 7, + Rematerializable = 1ULL << 8, // TODO: implies HasDest + MayRaiseError = 1ULL << 9, + Terminal = 1ULL << 10, // has no next instruction + NaryDest = 1ULL << 11, // has 0 or more destinations + HasExtra = 1ULL << 12, + Passthrough = 1ULL << 13, + KillsSources = 1ULL << 14, + ModifiesStack = 1ULL << 15, + HasStackVersion = 1ULL << 16, +}; + +bool opcodeHasFlags(Opcode opc, uint64_t flags); +Opcode getStackModifyingOpcode(Opcode opc); + #define IRT_BOXES(name, bit) \ IRT(name, (bit)) \ IRT(Boxed##name, (bit) << kBoxShift) \ @@ -923,8 +956,9 @@ public: * value, which allows us to avoid several checks. */ bool isKnownDataType() const { - // Calling this function with a non-php type isn't meaningful - assert(subtypeOf(Gen)); + // Calling this function with a type that can't be in a TypedValue isn't + // meaningful + assert(subtypeOf(Gen | Cls)); // Str, Arr and Null are technically unions but can each be // represented by one data type. Same for a union that consists of // nothing but boxed types. @@ -1479,6 +1513,11 @@ struct IRInstruction { m_dst = newDst; m_numDsts = newDst ? 1 : 0; } + /* + * Returns the ith dest of this instruction. i == 0 is treated specially: if + * the instruction has no dests, getDst(0) will return nullptr, and if the + * instruction is not naryDest, getDst(0) will return the single dest. + */ SSATmp* getDst(unsigned i) const; DstRange getDsts(); Range getDsts() const; @@ -1559,6 +1598,12 @@ struct IRInstruction { bool killsSources() const; bool killsSource(int srcNo) const; + bool modifiesStack() const; + SSATmp* modifiedStkPtr() const; + // hasMainDst provides raw access to the HasDest flag, for instructions with + // ModifiesStack set. + bool hasMainDst() const; + void printDst(std::ostream& ostream) const; void printSrc(std::ostream& ostream, uint32_t srcIndex) const; void printOpcode(std::ostream& ostream) const; @@ -1595,10 +1640,10 @@ typedef boost::intrusive::list /* * Return the output type from a given IRInstruction. * - * The destination type is always predictable from the types of the - * inputs and any type parameters to the instruction. + * The destination type is always predictable from the types of the inputs, any + * type parameters to the instruction, and the id of the dest. */ -Type outputType(const IRInstruction*); +Type outputType(const IRInstruction*, int dstId = 0); /* * Assert that an instruction has operands of allowed types. @@ -1762,10 +1807,10 @@ private: // May only be created via IRFactory. Note that this class is never // destructed, so don't add complex members. - SSATmp(uint32_t opndId, IRInstruction* i) + SSATmp(uint32_t opndId, IRInstruction* i, int dstId = 0) : m_inst(i) , m_id(opndId) - , m_type(outputType(i)) + , m_type(outputType(i, dstId)) , m_lastUseId(0) , m_useCount(0) , m_isSpilled(false) @@ -1797,12 +1842,24 @@ private: }; }; -int vectorBaseIdx(const IRInstruction* inst); -int vectorKeyIdx(const IRInstruction* inst); -int vectorValIdx(const IRInstruction* inst); +int vectorBaseIdx(Opcode opc); +int vectorKeyIdx(Opcode opc); +int vectorValIdx(Opcode opc); +inline int vectorBaseIdx(const IRInstruction* inst) { + return vectorBaseIdx(inst->getOpcode()); +} +inline int vectorKeyIdx(const IRInstruction* inst) { + return vectorKeyIdx(inst->getOpcode()); +} +inline int vectorValIdx(const IRInstruction* inst) { + return vectorValIdx(inst->getOpcode()); +} struct VectorEffects { - static bool supported(const IRInstruction* inst); + static bool supported(Opcode op); + static bool supported(const IRInstruction* inst) { + return supported(inst->getOpcode()); + } /* * VectorEffects::get is used to allow multiple different consumers to deal @@ -1820,6 +1877,16 @@ struct VectorEffects { StoreLocFunc storeLocValue, SetLocTypeFunc setLocType); + /* + * Given a vector instruction that defines a new StkPtr, attempts to find the + * stack value matching the given index. If the index does not match a value + * modified by this instruction, false is returned and the 'value' parameter + * is set to the next StkPtr in the chain. Otherwise, true is returned and + * the 'value' and 'type' parameters are set appropriately. + */ + static bool getStackValue(const IRInstruction* inst, uint32_t index, + SSATmp*& value, Type& type); + VectorEffects(const IRInstruction* inst) { int keyIdx = vectorKeyIdx(inst); int valIdx = vectorValIdx(inst); @@ -1828,6 +1895,15 @@ struct VectorEffects { keyIdx == -1 ? Type::None : inst->getSrc(keyIdx)->getType(), valIdx == -1 ? Type::None : inst->getSrc(valIdx)->getType()); } + template + VectorEffects(Opcode opc, const Container& srcs) { + int keyIdx = vectorKeyIdx(opc); + int valIdx = vectorValIdx(opc); + init(opc, + srcs[vectorBaseIdx(opc)]->getType(), + keyIdx == -1 ? Type::None : srcs[keyIdx]->getType(), + valIdx == -1 ? Type::None : srcs[valIdx]->getType()); + } VectorEffects(Opcode op, Type base, Type key, Type val) { init(op, base, key, val); } diff --git a/hphp/runtime/vm/translator/hopt/irfactory.h b/hphp/runtime/vm/translator/hopt/irfactory.h index 44211cc38..baa3edfd6 100644 --- a/hphp/runtime/vm/translator/hopt/irfactory.h +++ b/hphp/runtime/vm/translator/hopt/irfactory.h @@ -157,7 +157,19 @@ public: T* cloneInstruction(const T* inst) { T* newInst = new (m_arena) T(m_arena, inst, IRInstruction::IId(m_nextInstId++)); - newSSATmp(newInst); + if (newInst->modifiesStack()) { + assert(newInst->naryDst()); + // The instruction is an opcode that modifies the stack, returning a new + // StkPtr. + int numDsts = 1 + (newInst->hasMainDst() ? 1 : 0); + SSATmp* dsts = (SSATmp*)m_arena.alloc(numDsts * sizeof(SSATmp)); + for (int dstNo = 0; dstNo < numDsts; ++dstNo) { + new (&dsts[dstNo]) SSATmp(m_nextOpndId++, newInst, dstNo); + } + newInst->setDsts(numDsts, dsts); + } else { + newSSATmp(newInst); + } return newInst; } diff --git a/hphp/runtime/vm/translator/hopt/linearscan.cpp b/hphp/runtime/vm/translator/hopt/linearscan.cpp index a9607114d..205dce4dc 100644 --- a/hphp/runtime/vm/translator/hopt/linearscan.cpp +++ b/hphp/runtime/vm/translator/hopt/linearscan.cpp @@ -319,29 +319,30 @@ void LinearScan::allocRegToInstruction(InstructionList::iterator it) { allocRegToTmp(&m_regs[int(rVmFp)], &dsts[0], 0); return; } - if (dsts[0].isA(Type::StkPtr) && opc != LdRaw) { - assert(opc == DefSP || opc == Call || opc == SpillStack || - opc == RetAdjustStack || - opc == NewObj || opc == InterpOne || opc == GenericRetDecRefs || - opc == GuardStk || opc == AssertStk || opc == CastStk || - opc == SetProp || opc == SetElem); - allocRegToTmp(&m_regs[int(rVmSp)], &dsts[0], 0); - return; - } if (opc == DefMIStateBase) { assert(dsts[0].isA(Type::PtrToCell)); allocRegToTmp(&m_regs[int(rsp)], &dsts[0], 0); return; } - // LdRaw, loading a generator's embedded AR, is the only time we have a - // pointer to an AR that is not in rVmFp or rVmSp. - assert(!dsts[0].isA(Type::StkPtr) || - (opc == LdRaw && - inst->getSrc(1)->getValInt() == RawMemSlot::ContARPtr)); - for (SSATmp& dst : dsts) { for (int i = 0, n = dst.numNeededRegs(); i < n; ++i) { + if (dst.isA(Type::StkPtr) && opc != LdRaw) { + assert(opc == DefSP || opc == Call || opc == SpillStack || + opc == RetAdjustStack || + opc == NewObj || opc == InterpOne || opc == GenericRetDecRefs || + opc == GuardStk || opc == AssertStk || opc == CastStk || + VectorEffects::supported(opc)); + allocRegToTmp(&m_regs[int(rVmSp)], &dst, 0); + continue; + } + + // LdRaw, loading a generator's embedded AR, is the only time we have a + // pointer to an AR that is not in rVmFp or rVmSp. + assert(!dst.isA(Type::StkPtr) || + (opc == LdRaw && + inst->getSrc(1)->getValInt() == RawMemSlot::ContARPtr)); + if (!RuntimeOption::EvalHHIRDeadCodeElim || dst.getLastUseId() != 0) { allocRegToTmp(&dst, i); } diff --git a/hphp/runtime/vm/translator/hopt/memelim.cpp b/hphp/runtime/vm/translator/hopt/memelim.cpp index 4fee82a72..6fea1cf5c 100644 --- a/hphp/runtime/vm/translator/hopt/memelim.cpp +++ b/hphp/runtime/vm/translator/hopt/memelim.cpp @@ -264,15 +264,7 @@ private: } static bool isVectorOp(Opcode opc) { - switch (opc) { - case SetProp: - case SetElem: - case SetNewElem: - case ElemDX: - return true; - default: - return false; - } + return VectorEffects::supported(opc); } bool isLive(IRInstruction* inst) const { diff --git a/hphp/runtime/vm/translator/hopt/nativecalls.cpp b/hphp/runtime/vm/translator/hopt/nativecalls.cpp index 74b3e282f..188811828 100644 --- a/hphp/runtime/vm/translator/hopt/nativecalls.cpp +++ b/hphp/runtime/vm/translator/hopt/nativecalls.cpp @@ -130,6 +130,16 @@ static CallMap s_callMap({ CallMap::CallMap(CallInfoList infos) { for (auto const& info : infos) { m_map[info.op] = info; + + // Check for opcodes that have a version which modifies the stack, + // and add an entry to the table for that one. + if (opcodeHasFlags(info.op, HasStackVersion)) { + Opcode stkOp = getStackModifyingOpcode(info.op); + assert(opcodeHasFlags(stkOp, ModifiesStack)); + auto& slot = m_map[stkOp]; + slot = info; + slot.op = stkOp; + } } } diff --git a/hphp/runtime/vm/translator/hopt/opt.cpp b/hphp/runtime/vm/translator/hopt/opt.cpp index a06eac285..ced9b8391 100644 --- a/hphp/runtime/vm/translator/hopt/opt.cpp +++ b/hphp/runtime/vm/translator/hopt/opt.cpp @@ -60,8 +60,8 @@ static void insertSpillStackAsserts(IRInstruction& inst, IRFactory* factory) { i += kSpillStackActRecExtraArgs; } else { if (t.subtypeOf(Type::Gen)) { - IRInstruction* addr = factory->gen(LdStackAddr, sp, - factory->defConst(offset)); + IRInstruction* addr = factory->gen(LdStackAddr, Type::PtrToGen, + sp, factory->defConst(offset)); block->insert(pos, addr); IRInstruction* check = factory->gen(DbgAssertPtr, addr->getDst()); block->insert(pos, check); @@ -86,7 +86,8 @@ static void insertAsserts(Trace* trace, IRFactory* factory) { } if (inst.getOpcode() == Call) { SSATmp* sp = inst.getDst(); - IRInstruction* addr = factory->gen(LdStackAddr, sp, factory->defConst(0)); + IRInstruction* addr = factory->gen(LdStackAddr, Type::PtrToGen, + sp, factory->defConst(0)); insertAfter(&inst, addr); insertAfter(addr, factory->gen(DbgAssertPtr, addr->getDst())); continue; diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index 011f7be66..b12fc576f 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -72,6 +72,7 @@ static bool isNotInst(SSATmp *tmp) { SSATmp* Simplifier::simplify(IRInstruction* inst) { SSATmp* src1 = inst->getSrc(0); SSATmp* src2 = inst->getSrc(1); + Opcode opc = inst->getOpcode(); switch (opc) { case OpAdd: return simplifyAdd(src1, src2); diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index c397b2137..c064ca0f6 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -593,16 +593,9 @@ SSATmp* TraceBuilder::genLdLocAsCell(uint32_t id, Trace* exitTrace) { return genLdRef(tmp, type.innerType(), exitTrace); } -SSATmp* TraceBuilder::genLdLocAddr(uint32_t id, Trace* exitTrace) { +SSATmp* TraceBuilder::genLdLocAddr(uint32_t id) { LocalId baseLocalId(id); - Type t = getLocalType(id); - if (exitTrace) { - // If we have an exitTrace, emit a LdRef for its guard side-effect. - assert(t.isBoxed()); - SSATmp* locVal = genLdLoc(id); - gen(LdRef, t.unbox(), exitTrace, locVal); - } - return gen(LdLocAddr, t.ptr(), &baseLocalId, getFp()); + return gen(LdLocAddr, getLocalType(id).ptr(), &baseLocalId, getFp()); } void TraceBuilder::genStLocAux(uint32_t id, SSATmp* newValue, bool storeType) { @@ -887,13 +880,21 @@ static SSATmp* getStackValue(SSATmp* sp, type); } - default: - break; + default: { + SSATmp* value; + if (VectorEffects::getStackValue(sp->getInstruction(), index, + value, type)) { + return value; + } else { + // If VectorEffects::getStackValue failed, it returns the next + // sp to search in value. + return getStackValue(value, index, spansCall, type); + } + } } // Should not get here! - assert(0); - return nullptr; + not_reached(); } void TraceBuilder::genAssertStk(uint32_t id, Type type) { @@ -927,8 +928,14 @@ SSATmp* TraceBuilder::genDefSP(int32_t spOffset) { return gen(DefSP, m_fpValue, genDefConst(spOffset)); } -SSATmp* TraceBuilder::genLdStackAddr(int64_t index) { - return gen(LdStackAddr, m_spValue, genDefConst(index)); +SSATmp* TraceBuilder::genLdStackAddr(SSATmp* sp, int64_t index) { + Type type; + bool spansCall; + UNUSED SSATmp* val = getStackValue(sp, index, spansCall, type); + type = type.subtypeOf(Type::None) ? Type::Gen : type; + assert(IMPLIES(val != nullptr, val->getType().equals(type))); + assert(type.notPtr()); + return gen(LdStackAddr, type.ptr(), sp, genDefConst(index)); } void TraceBuilder::genNativeImpl() { @@ -1255,7 +1262,6 @@ void TraceBuilder::updateTrackedState(IRInstruction* inst) { } if (VectorEffects::supported(inst)) { - // TODO: Handle stack cells at some point. t1961007 VectorEffects::get(inst, [&](uint32_t id, SSATmp* val) { // storeLocalValue setLocalValue(id, val); @@ -1265,6 +1271,10 @@ void TraceBuilder::updateTrackedState(IRInstruction* inst) { }); } + if (inst->modifiesStack()) { + m_spValue = inst->modifiedStkPtr(); + } + // update the CSE table if (m_enableCse && inst->canCSE()) { cseInsert(inst); @@ -1467,8 +1477,9 @@ SSATmp* TraceBuilder::optimizeInst(IRInstruction* inst) { if (inst->getOpcode() != Nop) { inst = inst->clone(&m_irFactory); appendInstruction(inst); - // returns nullptr if instruction has no dest or multiple dests - return inst->hasDst() ? inst->getDst() : nullptr; + // returns nullptr if instruction has no dest, returns the first + // (possibly only) dest otherwise + return inst->getDst(0); } return nullptr; } diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.h b/hphp/runtime/vm/translator/hopt/tracebuilder.h index 3c17e43b6..8221ba50c 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.h +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.h @@ -85,7 +85,7 @@ public: void genStRaw(SSATmp* base, RawMemSlot::Kind kind, SSATmp* value); SSATmp* genLdLoc(uint32_t id); - SSATmp* genLdLocAddr(uint32_t id, Trace* exitTrace = nullptr); + SSATmp* genLdLocAddr(uint32_t id); void genRaiseUninitLoc(uint32_t id); /* @@ -209,7 +209,10 @@ public: SSATmp* genLdStack(int32_t stackOff, Type type); SSATmp* genDefFP(); SSATmp* genDefSP(int32_t spOffset); - SSATmp* genLdStackAddr(int64_t offset); + SSATmp* genLdStackAddr(SSATmp* sp, int64_t offset); + SSATmp* genLdStackAddr(int64_t offset) { + return genLdStackAddr(m_spValue, offset); + } void genVerifyParamType(SSATmp* objClass, SSATmp* className, const Class* constraint, Trace* exitTrace); diff --git a/hphp/runtime/vm/translator/hopt/type.cpp b/hphp/runtime/vm/translator/hopt/type.cpp index e9567e33c..fbca83006 100644 --- a/hphp/runtime/vm/translator/hopt/type.cpp +++ b/hphp/runtime/vm/translator/hopt/type.cpp @@ -59,14 +59,27 @@ Type boxReturn(const IRInstruction* inst, int srcId) { } -Type outputType(const IRInstruction* inst) { +Type stkReturn(const IRInstruction* inst, int dstId, + std::function inner) { + assert(inst->modifiesStack()); + if (dstId == 0 && inst->hasMainDst()) { + // Return the type of the main dest (if one exists) as dst 0 + return inner(); + } + // The instruction modifies the stack and this isn't the main dest, + // so it's a StkPtr. + return Type::StkPtr; +} + +Type outputType(const IRInstruction* inst, int dstId) { #define D(type) return Type::type; #define DofS(n) return inst->getSrc(n)->getType(); #define DUnbox(n) return inst->getSrc(n)->getType().unbox(); #define DBox(n) return boxReturn(inst, n); #define DParam return inst->getTypeParam(); -#define DLabel return Type::None; +#define DMulti return Type::None; +#define DStk(in) return stkReturn(inst, dstId, [&]{ in not_reached(); }); #define DVector return vectorReturn(inst); #define ND assert(0 && "outputType requires HasDest or NaryDest"); #define DBuiltin return builtinReturn(inst); @@ -85,7 +98,8 @@ Type outputType(const IRInstruction* inst) { #undef DUnbox #undef DBox #undef DParam -#undef DLabel +#undef DMulti +#undef DStk #undef DVector #undef ND #undef DBuiltin @@ -213,7 +227,8 @@ void assertOperandTypes(const IRInstruction* inst) { #define SNum S(Int, Bool, Dbl) #define SUnk return; #define ND -#define DLabel +#define DMulti +#define DStk(...) #define DVector #define D(...) #define DBuiltin @@ -246,6 +261,8 @@ void assertOperandTypes(const IRInstruction* inst) { #undef ND #undef D #undef DUnbox +#undef DMulti +#undef DStk #undef DBox #undef DofS #undef DParam diff --git a/hphp/runtime/vm/translator/hopt/vectortranslator.cpp b/hphp/runtime/vm/translator/hopt/vectortranslator.cpp index bdfac6ab5..e09f2d49f 100644 --- a/hphp/runtime/vm/translator/hopt/vectortranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/vectortranslator.cpp @@ -31,30 +31,12 @@ TRACE_SET_MOD(hhir); using Transl::MInstrState; using Transl::mInstrHasUnknownOffsets; -namespace { - -// Reduce baseType to a canonical unboxed, non-pointer form, returning (in -// basePtr and baseBoxed) whether the original type was a pointer or boxed, -// respectively. Whether or not the type is a pointer and/or boxed must be -// statically known, unless it's exactly equal to Gen. -void stripBase(Type& baseType, bool& basePtr, bool& baseBoxed) { - assert_not_implemented(baseType.isPtr() || baseType.notPtr()); - basePtr = baseType.isPtr(); - baseType = basePtr ? baseType.deref() : baseType; - assert_not_implemented( - baseType.equals(Type::Gen) || baseType.isBoxed() || baseType.notBoxed()); - baseBoxed = baseType.isBoxed(); - baseType = baseBoxed ? baseType.innerType() : baseType; -} - -} - -bool VectorEffects::supported(const IRInstruction* inst) { - switch (inst->getOpcode()) { - case SetProp: - case SetElem: - case SetNewElem: - case ElemDX: +bool VectorEffects::supported(Opcode op) { + switch (op) { + case SetProp: case SetPropStk: + case SetElem: case SetElemStk: + case SetNewElem: case SetNewElemStk: + case ElemDX: case ElemDXStk: return true; default: @@ -83,26 +65,61 @@ void VectorEffects::get(const IRInstruction* inst, } } +bool VectorEffects::getStackValue(const IRInstruction* inst, uint32_t index, + SSATmp*& value, Type& type) { + SSATmp* base = inst->getSrc(vectorBaseIdx(inst)); + assert(base->getInstruction()->getOpcode() == LdStackAddr); + if (base->getInstruction()->getSrc(1)->getValInt() != index) { + value = base->getInstruction()->getSrc(0); + assert(value->isA(Type::StkPtr)); + return false; + } + + VectorEffects ve(inst); + assert(ve.baseTypeChanged || ve.baseValChanged); + value = nullptr; + type = ve.baseType.derefIfPtr(); + return true; +} + +namespace { +// Reduce baseType to a canonical unboxed, non-pointer form, returning (in +// basePtr and baseBoxed) whether the original type was a pointer or boxed, +// respectively. Whether or not the type is a pointer and/or boxed must be +// statically known, unless it's exactly equal to Gen. +void stripBase(Type& baseType, bool& basePtr, bool& baseBoxed) { + assert_not_implemented(baseType.isPtr() || baseType.notPtr()); + basePtr = baseType.isPtr(); + baseType = basePtr ? baseType.deref() : baseType; + assert_not_implemented( + baseType.equals(Type::Gen) || baseType.isBoxed() || baseType.notBoxed()); + baseBoxed = baseType.isBoxed(); + baseType = baseBoxed ? baseType.innerType() : baseType; +} +} + void VectorEffects::init(Opcode op, const Type origBase, const Type key, const Type origVal) { baseType = origBase; bool basePtr, baseBoxed; stripBase(baseType, basePtr, baseBoxed); - // Every base coming through here should be a pointer or object for now, but - // may not be in the future. - assert_not_implemented(basePtr || baseType.subtypeOf(Type::Obj)); + // Only certain types of bases are supported now, but this list may expand in + // the future. + assert_not_implemented(basePtr || + baseType.subtypeOfAny(Type::Obj, Type::Arr)); valType = origVal; baseTypeChanged = baseValChanged = valTypeChanged = false; // Canonicalize the op to SetProp or SetElem switch (op) { - case SetProp: + case SetProp: case SetPropStk: break; - case SetElem: - case SetNewElem: - case ElemDX: + case SetElem: case SetElemStk: + case SetNewElem: case SetNewElemStk: + case ElemDX: case ElemDXStk: + case ArraySet: op = SetElem; break; @@ -125,13 +142,16 @@ void VectorEffects::init(Opcode op, const Type origBase, baseValChanged = true; } if (op == SetElem && baseType.maybe(Type::Arr)) { - // possible COW when modifying an array - baseValChanged = true; + // possible COW when modifying an array, unless the base was boxed. If the + // base is a box then the value of the box itself won't change, just what + // it points to. + baseValChanged = !baseBoxed; } if (op == SetElem && baseType.maybe(Type::StaticArr)) { - // the base might get promoted to a CountedArr + // the base might get promoted to a CountedArr. We can ignore the change if + // the base is boxed, (for the same reasons as above). baseType = baseType | Type::CountedArr; - baseValChanged = true; + baseValChanged = !baseBoxed; } // Setting an element with a base of one of these types will either succeed @@ -169,34 +189,37 @@ void VectorEffects::init(Opcode op, const Type origBase, } // vectorBaseIdx returns the src index for inst's base operand. -int vectorBaseIdx(const IRInstruction* inst) { - switch (inst->getOpcode()) { - case SetProp: return 2; - case SetElem: return 1; - case ElemDX: return 1; - case SetNewElem: return 0; +int vectorBaseIdx(Opcode opc) { + switch (opc) { + case SetProp: case SetPropStk: return 2; + case SetElem: case SetElemStk: return 1; + case ElemDX: case ElemDXStk: return 1; + case SetNewElem: case SetNewElemStk: return 0; + case ArraySet: return 1; default: not_reached(); } } // vectorKeyIdx returns the src index for inst's key operand. -int vectorKeyIdx(const IRInstruction* inst) { - switch (inst->getOpcode()) { - case SetProp: return 3; - case SetElem: return 2; - case ElemDX: return 2; - case SetNewElem: return -1; +int vectorKeyIdx(Opcode opc) { + switch (opc) { + case SetProp: case SetPropStk: return 3; + case SetElem: case SetElemStk: return 2; + case ElemDX: case ElemDXStk: return 2; + case SetNewElem: case SetNewElemStk: return -1; + case ArraySet: return 2; default: not_reached(); } } // vectorValIdx returns the src index for inst's value operand. -int vectorValIdx(const IRInstruction* inst) { - switch (inst->getOpcode()) { - case SetProp: return 4; - case SetElem: return 3; - case ElemDX: return -1; - case SetNewElem: return 1; +int vectorValIdx(Opcode opc) { + switch (opc) { + case SetProp: case SetPropStk: return 4; + case SetElem: case SetElemStk: return 3; + case ElemDX: case ElemDXStk: return -1; + case SetNewElem: case SetNewElemStk: return 1; + case ArraySet: return 3; default: not_reached(); } } @@ -215,6 +238,35 @@ HhbcTranslator::VectorTranslator::VectorTranslator( { } +/* Copy varargs SSATmp*s into a vector */ +template +static void getSrcs(std::vector& srcVec, SSATmp* src, Srcs... srcs) { + srcVec.push_back(src); + getSrcs(srcVec, srcs...); +} +static void getSrcs(std::vector& srcVec) {} + +template +SSATmp* HhbcTranslator::VectorTranslator::genStk(Opcode opc, Srcs... srcs) { + assert(opcodeHasFlags(opc, HasStackVersion)); + assert(!opcodeHasFlags(opc, ModifiesStack)); + std::vector srcVec; + getSrcs(srcVec, srcs...); + SSATmp* base = srcVec[vectorBaseIdx(opc)]; + + /* If the base is a pointer to a stack cell and the operation might change + * its type and/or value, use the version of the opcode that returns a new + * StkPtr. */ + if (base->getInstruction()->getOpcode() == LdStackAddr) { + VectorEffects ve(opc, srcVec); + if (ve.baseTypeChanged || ve.baseValChanged) { + opc = getStackModifyingOpcode(opc); + } + } + + return m_tb.gen(opc, srcs...); +} + void HhbcTranslator::VectorTranslator::emit() { // Assign stack slots to our stack inputs numberStackInputs(); @@ -244,6 +296,7 @@ void HhbcTranslator::VectorTranslator::checkMIState() { Type baseType = Type::fromRuntimeType(baseRtt); const bool isCGetM = m_ni.mInstrOp() == OpCGetM; const bool isSetM = m_ni.mInstrOp() == OpSetM; + const bool isSingle = m_ni.immVecM.size() == 1; assert(baseType.isBoxed() || baseType.notBoxed()); baseType = baseType.unbox(); @@ -251,20 +304,23 @@ void HhbcTranslator::VectorTranslator::checkMIState() { // CGetM or SetM with no unknown property offsets const bool simpleProp = !mInstrHasUnknownOffsets(m_ni) && (isCGetM || isSetM); + // SetM with only one element + const bool singlePropSet = isSingle && isSetM && + mcodeMaybePropName(m_ni.immVecM[0]); + // Array access with one element in the vector - const bool simpleElem = m_ni.immVecM.size() == 1 && - mcodeMaybeArrayKey(m_ni.immVecM[0]); + const bool singleElem = isSingle && mcodeMaybeArrayKey(m_ni.immVecM[0]); // SetM on an array with one vector element - const bool simpleArraySet = isSetM && simpleElem; + const bool simpleArraySet = isSetM && singleElem; // CGetM on an array with a base that won't use MInstrState. Str // will use tvScratch and baseStrOff and Obj will fatal or use // tvRef. - const bool simpleArrayGet = isCGetM && simpleElem && + const bool simpleArrayGet = isCGetM && singleElem && baseType.not(Type::Str | Type::Obj); - if (simpleProp || simpleArraySet || simpleArrayGet) { + if (simpleProp || singlePropSet || simpleArraySet || simpleArrayGet) { setNoMIState(); } } @@ -370,68 +426,60 @@ SSATmp* HhbcTranslator::VectorTranslator::getInput(unsigned i) { void HhbcTranslator::VectorTranslator::emitBaseLCR() { const MInstrAttr& mia = m_mii.getAttr(m_ni.immVec.locationCode()); const DynLocation& base = *m_ni.inputs[m_iInd]; - if (mia & MIA_warn) { - if (base.rtt.isUninit()) { - m_ht.spillStack(); - LocalId data(base.location.offset); - m_tb.gen(RaiseUninitLoc, &data); + auto baseType = Type::fromRuntimeType(base.rtt); + assert(baseType.isKnownDataType()); + + if (baseType.subtypeOfAny(Type::Null, Type::BoxedNull)) { + PUNT(emitBaseLCR-NullBase); + } + if (base.location.isLocal()) { + // Check for Uninit and warn/promote to InitNull as appropriate + if (baseType.subtypeOf(Type::Uninit)) { + if (mia & MIA_warn) { + m_ht.spillStack(); + LocalId data(base.location.offset); + m_tb.gen(RaiseUninitLoc, &data); + } + if (mia & MIA_define) { + m_tb.genStLoc(base.location.offset, m_tb.genDefInitNull(), + false, true, nullptr); + baseType = Type::InitNull; + } } } - if (base.location.isLocal()) { - uint32_t loc = base.location.offset; - auto baseType = m_tb.getLocalType(loc); - - if ((mia & MIA_define) && baseType.subtypeOf(Type::Uninit)) { - // Promote Uninit to InitNull before doing anything else - m_tb.genStLoc(loc, m_tb.genDefInitNull(), false, true, nullptr); - baseType = Type::InitNull; - } - if (baseType.unbox().isNull()) { - // The base local might get promoted to an Array or stdClass for set - // operations. Punt for now. - PUNT(emitBaseLCR-NullBase); - } - - // We should never have a union type for the base with mixed boxes - assert(baseType.isBoxed() || baseType.notBoxed()); - - // If the base is a box with a type that's changed, we need to bail out of - // the tracelet and retranslate. Doing an exit here is a little sketchy - // since we may have already emitted instructions with memory effects to - // initialize the MInstrState. These particular stores are harmless though, - // and the worst outcome here is that we'll ending up doing the stores - // twice, once for this instruction and once at the beginning of the - // retranslation. - Trace* failedRef = baseType.isBoxed() ? m_ht.getExitTrace() : nullptr; - if (baseType.unbox().subtypeOf(Type::Obj) && - mcodeMaybePropName(m_ni.immVecM[0])) { - // We can pass the base to helpers by value in this case - m_base = m_tb.genLdLoc(loc); - if (baseType.isBoxed()) { - assert(m_base->isA(Type::BoxedObj)); - m_base = m_tb.gen(LdRef, Type::Obj, failedRef, m_base); - } - assert(m_base->isA(Type::Obj)); - } else if (isSimpleArraySet()) { - m_base = m_tb.genLdLoc(loc); - if (baseType.isBoxed()) { - m_base = m_tb.gen(LdRef, Type::Arr, failedRef, m_base); - } - assert(m_base->isA(Type::Arr)); - } else { - // Everything else is passed by reference. We don't have to worry about - // unboxing here, since all the generic helpers understand boxed bases. - m_base = m_tb.genLdLocAddr(loc, failedRef); - assert(m_base->getType().isPtr()); - } + // If the base is a box with a type that's changed, we need to bail out of + // the tracelet and retranslate. Doing an exit here is a little sketchy since + // we may have already emitted instructions with memory effects to initialize + // the MInstrState. These particular stores are harmless though, and the + // worst outcome here is that we'll ending up doing the stores twice, once + // for this instruction and once at the beginning of the retranslation. + Trace* failedRef = baseType.isBoxed() ? m_ht.getExitTrace() : nullptr; + if ((baseType.subtypeOfAny(Type::Obj, Type::BoxedObj) && + mcodeMaybePropName(m_ni.immVecM[0])) || + isSimpleArraySet()) { + // In these cases we can pass the base by value, after unboxing if needed. + m_base = m_tb.gen(Unbox, failedRef, getInput(m_iInd)); + assert(m_base->isA(baseType.unbox())); } else { - // Stack bases require a little more stack magic than we have right - // now. Refcounting correctly is tricky too. - PUNT(emitBaseLCR-CellBase); - if (base.isVariant()) { - PUNT(emitBaseLCR-R); + // Everything else is passed by reference. We don't have to worry about + // unboxing here, since all the generic helpers understand boxed bases. + if (baseType.isBoxed()) { + SSATmp* box = getInput(m_iInd); + assert(box->isA(Type::BoxedCell)); + // Guard that the inner type hasn't changed + m_tb.gen(LdRef, baseType.innerType(), failedRef, box); } + + if (base.location.space == Location::Local) { + m_base = m_tb.genLdLocAddr(base.location.offset); + } else { + assert(base.location.space == Location::Stack); + m_ht.spillStack(); + assert(m_stackInputs.count(m_iInd)); + m_base = m_tb.genLdStackAddr(m_stackInputs[m_iInd]); + } + assert(m_base->getType().isPtr()); } } @@ -836,8 +884,11 @@ void HhbcTranslator::VectorTranslator::emitElem() { typedef TypedValue* (*OpFunc)(TypedValue*, TypedValue, MInstrState*); BUILD_OPTAB_HOT(getKeyTypeIS(key), key->isBoxed(), mia); m_ht.spillStack(); - m_base = m_tb.gen(mia & Define ? ElemDX : ElemX, - cns((TCA)opFunc), ptr(m_base), key, genMisPtr()); + if (mia & Define) { + m_base = genStk(ElemDX, cns((TCA)opFunc), ptr(m_base), key, genMisPtr()); + } else { + m_base = m_tb.gen(ElemX, cns((TCA)opFunc), ptr(m_base), key, genMisPtr()); + } } #undef HELPER_TABLE @@ -1039,9 +1090,8 @@ void HhbcTranslator::VectorTranslator::emitSetProp() { SSATmp* key = getInput(m_iInd); BUILD_OPTAB(getKeyTypeS(key), key->isBoxed(), m_base->isA(Type::Obj)); m_ht.spillStack(); - SSATmp* result = - m_tb.gen(SetProp, cns((TCA)opFunc), CTX(), - objOrPtr(m_base), noLitInt(key), value); + SSATmp* result = genStk(SetProp, cns((TCA)opFunc), CTX(), + objOrPtr(m_base), noLitInt(key), value); VectorEffects ve(result->getInstruction()); m_result = ve.valTypeChanged ? result : value; } @@ -1233,6 +1283,8 @@ HELPER_TABLE_ARRAY_SET(ELEM) void HhbcTranslator::VectorTranslator::emitSimpleArraySet(SSATmp* key, SSATmp* value) { + assert(m_iInd == m_mii.valCount() + 1); + const int baseStkIdx = m_mii.valCount(); assert(key->getType().notBoxed()); assert(value->getType().notBoxed()); bool checkForInt = false; @@ -1262,16 +1314,12 @@ void HhbcTranslator::VectorTranslator::emitSimpleArraySet(SSATmp* key, // call destructors so it has a sync point in nativecalls.cpp, but exceptions // are swallowed at destructor boundaries right now: #2182869. if (setRef) { - SSATmp* box; - if (base.location.space == Location::Local) { - box = m_tb.genLdLoc(base.location.offset); - } else if (base.location.space == Location::Stack) { - not_implemented(); - } else { - not_reached(); - } - + assert(base.location.space == Location::Local || + base.location.space == Location::Stack); + SSATmp* box = getInput(baseStkIdx); m_tb.gen(ArraySetRef, cns((TCA)opFunc), m_base, key, value, box); + // Unlike the non-ref case, we don't need to do anything to the stack + // because any load of the box will be guarded. } else { SSATmp* newArr = m_tb.gen(ArraySet, cns((TCA)opFunc), m_base, key, value); @@ -1279,7 +1327,11 @@ void HhbcTranslator::VectorTranslator::emitSimpleArraySet(SSATmp* key, if (base.location.space == Location::Local) { m_tb.genStLoc(base.location.offset, newArr, false, false, nullptr); } else if (base.location.space == Location::Stack) { - not_implemented(); + VectorEffects ve(newArr->getInstruction()); + assert(ve.baseValChanged); + assert(ve.baseType.subtypeOf(Type::Arr)); + m_ht.extendStack(baseStkIdx, Type::Gen); + m_ht.replace(baseStkIdx, newArr); } else { not_reached(); } @@ -1302,7 +1354,7 @@ void HhbcTranslator::VectorTranslator::emitSetElem() { typedef TypedValue (*OpFunc)(TypedValue*, TypedValue, Cell); BUILD_OPTAB_HOT(getKeyTypeIS(key), key->isBoxed()); m_ht.spillStack(); - SSATmp* result = m_tb.gen(SetElem, cns((TCA)opFunc), ptr(m_base), key, value); + SSATmp* result = genStk(SetElem, cns((TCA)opFunc), ptr(m_base), key, value); VectorEffects ve(result->getInstruction()); m_result = ve.valTypeChanged ? result : value; } diff --git a/hphp/test/vm/vector-stack-base.php b/hphp/test/vm/vector-stack-base.php new file mode 100644 index 000000000..b57181942 --- /dev/null +++ b/hphp/test/vm/vector-stack-base.php @@ -0,0 +1,68 @@ +n = self::$idx++; + printf("dumper %d constructing\n", $this->n); + } + function __destruct() { + printf("dumper %d destructing\n", $this->n); + var_dump($this); + } +} + +function makeObj() { + return new dumper; +} + +function &makeObjRef() { + return new dumper(); +} + +function makeArr() { + $a = array(); + $a['dumper'] = new dumper; + return $a; +} + +function &makeArrRef() { + static $a = array(); + var_dump($a); + $a['dumper'] = new dumper; + return $a; +} + +function main() { + echo "Entering main\n"; + // SetM with array base on the stack + makeArr()['dumper'] = new stdclass; + makeArrRef()['dumper'] = 24; + makeArrRef()[234] = 234; + + // More complex SetM with array base on the stack, then with a ref + makeArr()['dumper']->prop = null; + makeArrRef()['dumper']->propp = null; + makeArrRef()['dumper']->proppp = null; + + + // Clear out the reference to destruct the array + $ref =& makeArrRef(); + var_dump($ref); + $ref = null; + + makeObj()->prop = 'foo'; + var_dump(makeObj()->prop); + makeObjRef()->prop = 'bar'; + var_dump(makeObjRef()->prop[2]); + + echo "Done with main\n"; +} + +echo "Calling main\n"; +main(); +echo "Main has returned\n"; + diff --git a/hphp/test/vm/vector-stack-base.php.exp b/hphp/test/vm/vector-stack-base.php.exp new file mode 100644 index 000000000..c3b2f0a64 --- /dev/null +++ b/hphp/test/vm/vector-stack-base.php.exp @@ -0,0 +1,152 @@ +Calling main +Entering main +dumper 0 constructing +dumper 0 destructing +object(dumper)#1 (2) { + ["n":"dumper":private]=> + int(0) + ["prop"]=> + string(13) "default value" +} +array(0) { +} +dumper 1 constructing +dumper 1 destructing +object(dumper)#2 (2) { + ["n":"dumper":private]=> + int(1) + ["prop"]=> + string(13) "default value" +} +array(1) { + ["dumper"]=> + int(24) +} +dumper 2 constructing +dumper 3 constructing +dumper 3 destructing +object(dumper)#3 (2) { + ["n":"dumper":private]=> + int(3) + ["prop"]=> + NULL +} +array(2) { + ["dumper"]=> + object(dumper)#2 (2) { + ["n":"dumper":private]=> + int(2) + ["prop"]=> + string(13) "default value" + } + [234]=> + int(234) +} +dumper 4 constructing +dumper 2 destructing +object(dumper)#2 (2) { + ["n":"dumper":private]=> + int(2) + ["prop"]=> + string(13) "default value" +} +array(2) { + ["dumper"]=> + object(dumper)#3 (3) { + ["n":"dumper":private]=> + int(4) + ["prop"]=> + string(13) "default value" + ["propp"]=> + NULL + } + [234]=> + int(234) +} +dumper 5 constructing +dumper 4 destructing +object(dumper)#3 (3) { + ["n":"dumper":private]=> + int(4) + ["prop"]=> + string(13) "default value" + ["propp"]=> + NULL +} +array(2) { + ["dumper"]=> + object(dumper)#4 (3) { + ["n":"dumper":private]=> + int(5) + ["prop"]=> + string(13) "default value" + ["proppp"]=> + NULL + } + [234]=> + int(234) +} +dumper 6 constructing +dumper 5 destructing +object(dumper)#4 (3) { + ["n":"dumper":private]=> + int(5) + ["prop"]=> + string(13) "default value" + ["proppp"]=> + NULL +} +array(2) { + ["dumper"]=> + object(dumper)#5 (2) { + ["n":"dumper":private]=> + int(6) + ["prop"]=> + string(13) "default value" + } + [234]=> + int(234) +} +dumper 6 destructing +object(dumper)#5 (2) { + ["n":"dumper":private]=> + int(6) + ["prop"]=> + string(13) "default value" +} +dumper 7 constructing +dumper 7 destructing +object(dumper)#5 (2) { + ["n":"dumper":private]=> + int(7) + ["prop"]=> + string(3) "foo" +} +dumper 8 constructing +dumper 8 destructing +object(dumper)#5 (2) { + ["n":"dumper":private]=> + int(8) + ["prop"]=> + string(13) "default value" +} +string(13) "default value" +dumper 9 constructing +dumper 9 destructing +object(dumper)#5 (2) { + ["n":"dumper":private]=> + int(9) + ["prop"]=> + string(3) "bar" +} +dumper 10 constructing +dumper 10 destructing +object(dumper)#5 (2) { + ["n":"dumper":private]=> + int(10) + ["prop"]=> + string(13) "default value" +} +string(1) "f" +Done with main +Main has returned diff --git a/hphp/util/assertions.h b/hphp/util/assertions.h index 742c6c798..f64b87a57 100644 --- a/hphp/util/assertions.h +++ b/hphp/util/assertions.h @@ -165,6 +165,14 @@ class FailedAssertion : public std::exception { #define assert_throw_log(e, l) #endif +const bool do_assert = +#ifdef NDEBUG + false +#else + true +#endif + ; + ////////////////////////////////////////////////////////////////////// #endif