diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index c43e213bd..eb79daa66 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -1275,7 +1275,24 @@ D:Obj = CreateCl S0:ConstCls S1:ConstInt S2:FramePtr S3:StkPtr object. Saves the $this and Func* from ActRec S2 into the object, and returns the object. -CreateCont +D:Obj = CreateCont S0:TCA S1:FramePtr S2:ConstBool S3:ConstFunc S4:ConstFunc + + Create a continuation object. Arguments: + + S0 - function pointer to C++ helper + S1 - current frame pointer + S2 - whether the CreateCont hhbc op had getArgs set to true (which means + the function body may call func_get_args, etc). + S3 - the "original" function + S4 - the generator function + +D:Obj = InlineCreateCont S0:{Obj,Null} + + Create a zero-argument continuation object. S0 is the this pointer + for the continuation if it was a method, otherwise null. Used to + weaken the use of the frame in CreateCont when inlining an 'outer' + generator function. + FillContLocals FillContThis diff --git a/hphp/idl/continuation.idl.json b/hphp/idl/continuation.idl.json index ac9282131..2d1cfd0b8 100644 --- a/hphp/idl/continuation.idl.json +++ b/hphp/idl/continuation.idl.json @@ -41,7 +41,7 @@ "Iterator", "Awaitable" ], - "footer": " public: void setCalledClass(CStrRef cls) {\n const_assert(!hhvm);\n m_called_class = cls;\n }\nprotected: virtual bool php_sleep(Variant &ret);\npublic:\n inline void preNext() {\n if (m_done) {\n throw_exception(Object(SystemLib::AllocExceptionObject(\n \"Continuation is already finished\")));\n }\n if (m_running) {\n throw_exception(Object(SystemLib::AllocExceptionObject(\n \"Continuation is already running\")));\n }\n m_running = true;\n ++m_index;\n }\n\n inline void startedCheck() {\n if (m_index < 0LL) {\n throw_exception(\n Object(SystemLib::AllocExceptionObject(\"Need to call next() first\")));\n }\n }\n\npublic:\n Object m_obj;\n Array m_args;\n int64_t m_index;\n Variant m_value;\n Variant m_received;\n String m_origFuncName;\n bool m_done;\n bool m_running;\n bool m_should_throw;\n\n int m_localsOffset;\n VM::Func *m_vmFunc;\n int64_t m_label\n VM::ActRec* m_arPtr;\n\n p_ContinuationWaitHandle m_waitHandle;\n\n SmartPtr m_VMStatics;\n\n String& getCalledClass() { not_reached(); }\n\n HphpArray* getStaticLocals();\n static size_t sizeForLocalsAndIters(int nLocals, int nIters) {\n return (sizeof(c_Continuation) + sizeof(TypedValue) * nLocals +\n sizeof(VM::Iter) * nIters + sizeof(VM::ActRec));\n }\n VM::ActRec* actRec() {\n return m_arPtr;\n }\n TypedValue* locals() {\n return (TypedValue*)(uintptr_t(this) + m_localsOffset);\n }", + "footer": "", "funcs": [ { "name": "__construct", @@ -49,24 +49,6 @@ "type": null }, "args": [ - { - "name": "func", - "type": "Int64" - }, - { - "name": "origFuncName", - "type": "String" - }, - { - "name": "obj", - "type": "Variant", - "value": "null" - }, - { - "name": "args", - "type": "VariantMap", - "value": "null_array" - } ], "flags": [ ] diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index 9ae590645..ea4d838e1 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -427,6 +427,13 @@ public: static void getElem(TypedValue* base, TypedValue* key, TypedValue* dest); template + static c_Continuation* createContinuationHelper( + const VM::Func* origFunc, + const VM::Func* genFunc, + ObjectData* thisPtr, + ArrayData* args, + VM::Class* frameStaticCls); + template static c_Continuation* createContinuation(ActRec* fp, bool getArgs, const VM::Func* origFunc, const VM::Func* genFunc); @@ -797,7 +804,6 @@ private: class Silencer { public: - Silencer() : m_active(false) {} explicit Silencer(bool); ~Silencer() { if (m_active) disableHelper(); } diff --git a/hphp/runtime/ext/ext_continuation.cpp b/hphp/runtime/ext/ext_continuation.cpp index a9d1e5515..ab2de3f06 100644 --- a/hphp/runtime/ext/ext_continuation.cpp +++ b/hphp/runtime/ext/ext_continuation.cpp @@ -69,20 +69,7 @@ c_Continuation::~c_Continuation() { } } -void c_Continuation::t___construct( - int64_t func, CStrRef origFuncName, CVarRef obj, CArrRef args) { - m_vmFunc = (VM::Func*)func; - assert(m_vmFunc); - m_origFuncName = origFuncName; - - if (!obj.isNull()) { - m_obj = obj.toObject(); - assert(!m_obj.isNull()); - } else { - assert(m_obj.isNull()); - } - m_args = args; -} +void c_Continuation::t___construct() {} void c_Continuation::t_update(int64_t label, CVarRef value) { m_label = label; @@ -156,24 +143,26 @@ void c_Continuation::t_raised() { String c_Continuation::t_getorigfuncname() { String called_class; + String origFunc(const_cast(m_origFuncName)); + if (actRec()->hasThis()) { called_class = actRec()->getThis()->getVMClass()->name()->data(); } else if (actRec()->hasClass()) { called_class = actRec()->getClass()->name()->data(); } if (called_class.size() == 0) { - return m_origFuncName; + return origFunc; } /* Replace the class name in m_origFuncName with the LSB class. This produces more useful traces. */ - size_t method_pos = m_origFuncName.find("::"); + size_t method_pos = origFunc.find("::"); if (method_pos != std::string::npos) { - return concat3(called_class, "::", m_origFuncName.substr(method_pos+2)); + return concat3(called_class, "::", origFunc.substr(method_pos+2)); } else { - return m_origFuncName; + return origFunc; } } diff --git a/hphp/runtime/ext/ext_continuation.h b/hphp/runtime/ext/ext_continuation.h index 34840da4c..a112a8880 100644 --- a/hphp/runtime/ext/ext_continuation.h +++ b/hphp/runtime/ext/ext_continuation.h @@ -42,11 +42,29 @@ class c_Continuation : public ExtObjectData { this_->m_vmFunc->numIterators()))(this_); } - // need to implement - public: explicit c_Continuation(VM::Class* cls = c_Continuation::s_cls); ~c_Continuation(); - void t___construct(int64_t func, CStrRef origFuncName, CVarRef obj = uninit_null(), CArrRef args = null_array); + +public: + void init(const VM::Func* vmFunc, + const StringData* origFuncName, + ObjectData* thisPtr, + ArrayData* args) noexcept { + m_vmFunc = const_cast(vmFunc); + assert(m_vmFunc); + m_origFuncName = origFuncName; + assert(m_origFuncName->isStatic()); + + if (thisPtr != nullptr) { + m_obj = thisPtr; + } else { + assert(m_obj.isNull()); + } + + m_args = args; + } + + void t___construct(); void t_update(int64_t label, CVarRef value); Object t_getwaithandle(); int64_t t_getlabel(); @@ -108,7 +126,7 @@ public: int64_t m_index; Variant m_value; Variant m_received; - String m_origFuncName; + const StringData* m_origFuncName; bool m_done; bool m_running; bool m_should_throw; diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index 3afc79cc0..1fb7de5e5 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -6667,53 +6667,40 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCreateCl(PC& pc) { template c_Continuation* -VMExecutionContext::createContinuation(ActRec* fp, - bool getArgs, - const Func* origFunc, - const Func* genFunc) { - Object obj; - Array args; - if (fp->hasThis()) { - obj = fp->getThis(); - } - if (getArgs) { - args = hhvm_get_frame_args(fp); - } - static const StringData* closure = StringData::GetStaticString("{closure}"); - const StringData* origName = - origFunc->isClosureBody() ? closure : origFunc->fullName(); - int nLocals = genFunc->numLocals(); - int nIters = genFunc->numIterators(); - Class* genClass = SystemLib::s_ContinuationClass; - c_Continuation* cont = c_Continuation::alloc(genClass, nLocals, nIters); +VMExecutionContext::createContinuationHelper(const Func* origFunc, + const Func* genFunc, + ObjectData* thisPtr, + ArrayData* args, + Class* frameStaticCls) { + auto const cont = c_Continuation::alloc( + SystemLib::s_ContinuationClass, + genFunc->numLocals(), + genFunc->numIterators() + ); cont->incRefCount(); cont->setNoDestruct(); - try { - cont->t___construct(uintptr_t(genFunc), - StrNR(const_cast(origName)), - obj, args); - } catch (...) { - decRefObj(cont); - throw; - } + + static auto const closureName = StringData::GetStaticString("{closure}"); + auto const origName = origFunc->isClosureBody() ? closureName + : origFunc->fullName(); + + cont->init(genFunc, origName, thisPtr, args); + // The ActRec corresponding to the generator body lives as long as the object // does. We set it up once, here, and then just change FP to point to it when // we enter the generator body. ActRec* ar = cont->actRec(); if (isMethod) { - Class* cls = frameStaticClass(fp); - if (origFunc->isClosureBody()) { - genFunc = genFunc->cloneAndSetClass(fp->m_func->cls()); + genFunc = genFunc->cloneAndSetClass(origFunc->cls()); } - if (obj.get()) { - ObjectData* objData = obj.get(); - ar->setThis(objData); - objData->incRefCount(); + if (thisPtr) { + ar->setThis(thisPtr); + thisPtr->incRefCount(); } else { - ar->setClass(cls); + ar->setClass(frameStaticCls); } } else { ar->setThis(nullptr); @@ -6734,6 +6721,28 @@ VMExecutionContext::createContinuation(ActRec* fp, return cont; } +template +c_Continuation* +VMExecutionContext::createContinuation(ActRec* fp, + bool getArgs, + const Func* origFunc, + const Func* genFunc) { + ObjectData* const thisPtr = fp->hasThis() ? fp->getThis() : nullptr; + + Array args; + if (getArgs) { + args = hhvm_get_frame_args(fp); + } + + return createContinuationHelper( + origFunc, + genFunc, + thisPtr, + args.get(), + frameStaticClass(fp) + ); +} + static inline void setContVar(const Func* genFunc, const StringData* name, TypedValue* src, @@ -6797,6 +6806,12 @@ VMExecutionContext::fillContinuationVars(ActRec* fp, return cont; } +// Explicitly instantiate for hhbctranslator.o +template c_Continuation* VMExecutionContext::createContinuation( + ActRec*, bool, const Func*, const Func*); +template c_Continuation* VMExecutionContext::createContinuation( + ActRec*, bool, const Func*, const Func*); + inline void OPTBLD_INLINE VMExecutionContext::iopCreateCont(PC& pc) { NEXT(); DECODE_IVA(getArgs); diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 5e009470f..e714bf92d 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -749,6 +749,9 @@ void CodeGenerator::cgCallNative(Asm& a, IRInstruction* inst) { case VecKeyIS: argGroup.vectorKeyIS(src); break; + case Immed: + always_assert(0 && "We can't generate a native call for this"); + break; } } @@ -3216,6 +3219,32 @@ void CodeGenerator::cgNewObjNoCtorCached(IRInstruction* inst) { args); } +void CodeGenerator::cgInlineCreateCont(IRInstruction* inst) { + auto const& data = *inst->getExtra(); + auto const helper = data.origFunc->isMethod() + ? &VMExecutionContext::createContinuationHelper + : &VMExecutionContext::createContinuationHelper; + + cgCallHelper( + m_as, + reinterpret_cast(helper), + inst->getDst(), + kSyncPoint, + ArgGroup() + .immPtr(data.origFunc) + .immPtr(data.genFunc) + .ssa(inst->getSrc(0)) + .immPtr(nullptr) // getArgs array + // Deliberately ignoring frameStaticClass parameter, because + // it's unused if we have a $this pointer, and we don't inline + // functions with a null $this. + ); + if (data.origFunc->isMethod()) { + // We can't support a null $this. + assert(inst->getSrc(0)->isA(Type::Obj)); + } +} + void CodeGenerator::cgCall(IRInstruction* inst) { SSATmp* actRec = inst->getSrc(0); SSATmp* returnBcOffset = inst->getSrc(1); diff --git a/hphp/runtime/vm/translator/hopt/codegen.h b/hphp/runtime/vm/translator/hopt/codegen.h index a7f02646c..8e9ec0bf5 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.h +++ b/hphp/runtime/vm/translator/hopt/codegen.h @@ -399,6 +399,8 @@ struct ArgGroup { return imm(uintptr_t(ptr)); } + ArgGroup& immPtr(std::nullptr_t) { return imm(0); } + ArgGroup& reg(PhysReg reg) { m_args.push_back(ArgDesc(ArgDesc::Reg, PhysReg(reg), -1)); return *this; diff --git a/hphp/runtime/vm/translator/hopt/dce.cpp b/hphp/runtime/vm/translator/hopt/dce.cpp index 0d10b5729..0cfcc0e85 100644 --- a/hphp/runtime/vm/translator/hopt/dce.cpp +++ b/hphp/runtime/vm/translator/hopt/dce.cpp @@ -61,7 +61,7 @@ struct DceFlags { */ void incWeakUse() { if (m_weakUseCount + 1 > kMaxWeakUseCount) { - always_assert(!"currently there's only one instruction " + always_assert(!"currently there's only two instructions " "using this machinery, so this shouldn't " "happen ... "); return; @@ -368,7 +368,7 @@ void sinkIncRefs(Trace* trace, IRFactory* irFactory, DceState& state) { * of a DefInlineFP. In this case we can kill both, which may allow * removing a SpillFrame as well. */ -void optimizeActRecs(Trace* trace, DceState& state) { +void optimizeActRecs(Trace* trace, DceState& state, IRFactory* factory) { FTRACE(5, "AR:vvvvvvvvvvvvvvvvvvvvv\n"); SCOPE_EXIT { FTRACE(5, "AR:^^^^^^^^^^^^^^^^^^^^^\n"); }; @@ -389,6 +389,22 @@ void optimizeActRecs(Trace* trace, DceState& state) { } break; + case CreateCont: + { + auto const frameInst = inst->getSrc(1)->inst(); + if (frameInst->op() == DefInlineFP) { + auto const spInst = frameInst->getSrc(0)->inst(); + if (spInst->op() == SpillFrame && + spInst->getSrc(3)->type().subtypeOfAny(Type::Obj, Type::Null)) { + FTRACE(5, "CreateCont ({}): weak use of frame {}\n", + inst->getIId(), + frameInst->getIId()); + state[frameInst].incWeakUse(); + } + } + } + break; + case InlineReturn: { auto frameUses = inst->getSrc(0)->getUseCount(); @@ -423,7 +439,7 @@ void optimizeActRecs(Trace* trace, DceState& state) { switch (inst->op()) { case DecRefKillThis: { - auto fp = inst->getSrc(1); + auto const fp = inst->getSrc(1); if (state[fp->inst()].isDead()) { FTRACE(5, "DecRefKillThis ({}) -> DecRef\n", inst->getIId()); inst->setOpcode(DecRef); @@ -433,9 +449,31 @@ void optimizeActRecs(Trace* trace, DceState& state) { } break; + case CreateCont: + { + auto const fp = inst->getSrc(1); + if (state[fp->inst()].isDead()) { + FTRACE(5, "CreateCont ({}) -> InlineCreateCont\n", inst->getIId()); + + CreateContData data; + data.origFunc = inst->getSrc(3)->getValFunc(); + data.genFunc = inst->getSrc(4)->getValFunc(); + + assert(fp->inst()->getSrc(0)->inst()->op() == SpillFrame); + auto const thisPtr = fp->inst()->getSrc(0)->inst()->getSrc(3); + factory->replace( + inst, + InlineCreateCont, + data, + thisPtr + ); + } + } + break; + case InlineReturn: { - auto fp = inst->getSrc(0); + auto const fp = inst->getSrc(0); if (state[fp->inst()].isDead()) { FTRACE(5, "InlineReturn ({}) setDead\n", inst->getIId()); state[inst].setDead(); @@ -591,7 +629,7 @@ void eliminateDeadCode(Trace* trace, IRFactory* irFactory) { // Optimize unused inlined activation records. It's not necessary // to look at non-main traces for this. - optimizeActRecs(trace, state); + optimizeActRecs(trace, state, irFactory); // now remove instructions whose id == DEAD removeDeadInstructions(trace, state); diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index 088969068..968ff8a4b 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -837,29 +837,37 @@ void HhbcTranslator::emitIterFree(uint32_t iterId) { void HhbcTranslator::emitCreateCont(bool getArgs, Id funNameStrId) { - /* Runtime-determined slow path punts to TranslatorX64 for now */ - m_tb->genExitOnVarEnv(getExitSlowTrace()); + m_tb->gen(ExitOnVarEnv, getExitSlowTrace()->front(), m_tb->getFp()); - const StringData* genName = lookupStringId(funNameStrId); - const Func* origFunc = getCurFunc(); - const Func* genFunc = origFunc->getGeneratorBody(genName); - int origLocals = origFunc->numLocals(); - int genLocals = genFunc->numLocals(); + auto const genName = lookupStringId(funNameStrId); + auto const origFunc = getCurFunc(); + auto const genFunc = origFunc->getGeneratorBody(genName); + auto const origLocals = origFunc->numLocals(); + auto const genLocals = genFunc->numLocals(); - TCA helper = origFunc->isMethod() ? - (TCA)&VMExecutionContext::createContinuation : - (TCA)&VMExecutionContext::createContinuation; - SSATmp* cont = m_tb->gen(CreateCont, cns(helper), m_tb->getFp(), - cns(getArgs), cns(origFunc), cns(genFunc)); + auto const cont = m_tb->gen( + CreateCont, + cns( + origFunc->isMethod() ? + (TCA)&VMExecutionContext::createContinuation : + (TCA)&VMExecutionContext::createContinuation + ), + m_tb->getFp(), + cns(getArgs), + cns(origFunc), + cns(genFunc) + ); TranslatorX64::ContParamMap params; if (origLocals <= TranslatorX64::kMaxInlineContLocals && TranslatorX64::mapContParams(params, origFunc, genFunc)) { - static const StringData* thisStr = StringData::GetStaticString("this"); + static auto const thisStr = StringData::GetStaticString("this"); Id thisId = kInvalidId; - bool fillThis = origFunc->isNonClosureMethod() && !origFunc->isStatic() && + const bool fillThis = origFunc->isNonClosureMethod() && + !origFunc->isStatic() && ((thisId = genFunc->lookupVarId(thisStr)) != kInvalidId) && (origFunc->lookupVarId(thisStr) == kInvalidId); + SSATmp* locals = m_tb->gen(LdContLocalsPtr, cont); for (int i = 0; i < origLocals; ++i) { SSATmp* loc = m_tb->genIncRef(m_tb->genLdAssertedLoc(i, Type::Gen)); diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index 83de42163..645efa092 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -633,6 +633,25 @@ void IRInstruction::convertToMov() { assert(m_numDsts == 1); } +void IRInstruction::become(IRFactory* factory, IRInstruction* other) { + assert(other->isTransient() || m_numDsts == other->m_numDsts); + assert(!canCSE()); + auto& arena = factory->arena(); + + // Copy all but m_iid, m_block, m_listNode, and don't clone + // dests---the whole point of become() is things still point to us. + m_op = other->m_op; + m_typeParam = other->m_typeParam; + m_id = other->m_id; + m_liveRegs = other->m_liveRegs; + m_taken = other->m_taken; + m_tca = other->m_tca; + m_numSrcs = other->m_numSrcs; + m_extra = other->m_extra ? cloneExtra(m_op, other->m_extra, arena) : nullptr; + m_srcs = new (arena) SSATmp*[m_numSrcs]; + std::copy(other->m_srcs, other->m_srcs + m_numSrcs, m_srcs); +} + IRInstruction* IRInstruction::clone(IRFactory* factory) const { return factory->cloneInstruction(this); } @@ -889,11 +908,10 @@ static void printConst(std::ostream& os, IRInstruction* inst) { os << "NamedEntity(" << ne << ")"; } else if (t.subtypeOf(Type::TCA)) { TCA tca = c->as(); - char* nameRaw = Util::getNativeFunctionName(tca); - SCOPE_EXIT { free(nameRaw); }; - std::string name(nameRaw); - boost::algorithm::trim(name); - os << folly::format("TCA: {}({})", tca, name); + auto name = Util::getNativeFunctionName(tca); + SCOPE_EXIT { free(name); }; + os << folly::format("TCA: {}({})", tca, + boost::trim_copy(std::string(name))); } else if (t.subtypeOf(Type::None)) { os << "None:" << c->as(); } else if (t.isPtr()) { diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index c0048b5b8..31d28e1a5 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -26,9 +26,16 @@ #include #include #include + #include #include #include +#include +#include + +#include "folly/Range.h" +#include "folly/Conv.h" + #include "util/asm-x64.h" #include "util/trace.h" #include "runtime/ext/ext_continuation.h" @@ -40,9 +47,6 @@ #include "runtime/base/types.h" #include "runtime/vm/func.h" #include "runtime/vm/class.h" -#include "folly/Range.h" -#include "folly/Conv.h" -#include namespace HPHP { // forward declaration @@ -430,6 +434,7 @@ O(CreateCont, D(Obj), C(TCA) \ C(Bool) \ C(Func) \ C(Func), E|N|Mem|PRc) \ +O(InlineCreateCont, D(Obj), S(Obj,Null), E|N|PRc) \ O(FillContLocals, ND, S(FramePtr) \ C(Func) \ C(Func) \ @@ -735,6 +740,11 @@ private: uintptr_t m_dataBits; }; +struct CreateContData : IRExtraData { + const Func* origFunc; + const Func* genFunc; +}; + /* * EdgeData is linked list node that tracks the set of Jmp_'s that pass values * to a particular block. Each such Jmp_ has one node, and the block points @@ -778,7 +788,6 @@ struct StackOffset : IRExtraData { int32_t offset; }; - struct BCOffset : IRExtraData { explicit BCOffset(Offset offset) : offset(offset) {} @@ -817,6 +826,7 @@ X(ReDefSP, StackOffset); X(ReDefGeneratorSP, StackOffset); X(DefSP, StackOffset); X(DefInlineFP, BCOffset); +X(InlineCreateCont, CreateContData); #undef X @@ -1733,9 +1743,29 @@ struct IRInstruction { /* * Replace an instruction in place with a Mov. Used when we have * proven that the instruction's side effects are not needed. + * + * TODO: replace with become */ void convertToMov(); + /* + * Turns this instruction into the target instruction, without + * changing stable fields (IId, current block, list fields). The + * existing destination SSATmp(s) will continue to think they came + * from this instruction. + * + * The target instruction may be transient---we'll clone anything we + * need to keep, using factory for any needed memory. + * + * Note: if you want to use this to replace a CSE-able instruction + * you're probably going to have a bad time. For now it's a + * precondition that the current instruction can't CSE. + * + * Pre: other->isTransient() || numDsts() == other->numDsts() + * Pre: !canCSE() + */ + void become(IRFactory*, IRInstruction* other); + /* * Deep-copy an IRInstruction, using factory to allocate memory for * the IRInstruction itself, and its srcs/dests. @@ -1767,6 +1797,7 @@ 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 diff --git a/hphp/runtime/vm/translator/hopt/irfactory.h b/hphp/runtime/vm/translator/hopt/irfactory.h index dca50adaf..9919abd55 100644 --- a/hphp/runtime/vm/translator/hopt/irfactory.h +++ b/hphp/runtime/vm/translator/hopt/irfactory.h @@ -178,10 +178,28 @@ public: ); } - template - T* cloneInstruction(const T* inst) { - T* newInst = new (m_arena) T(m_arena, inst, - IRInstruction::IId(m_nextInstId++)); + /* + * Replace an existing IRInstruction with a new one. + * + * This may involve making more allocations in the arena, but the + * actual IRInstruction* itself, its IId, etc, will stay unchanged. + * + * This function takes arguments in the same format as gen(). + */ + template + void replace(IRInstruction* old, Args... args) { + makeInstruction( + [&] (IRInstruction* replacement) { old->become(this, replacement); }, + args... + ); + } + + /* + * Clone an instruction and its sources. + */ + IRInstruction* cloneInstruction(const IRInstruction* inst) { + auto newInst = new (m_arena) IRInstruction( + m_arena, inst, IRInstruction::IId(m_nextInstId++)); if (newInst->modifiesStack()) { assert(newInst->naryDst()); // The instruction is an opcode that modifies the stack, returning a new @@ -198,6 +216,9 @@ public: return newInst; } + /* + * Some helpers for creating specific instruction patterns. + */ IRInstruction* defLabel(); IRInstruction* defLabel(unsigned numDst); template SSATmp* defConst(T val) { diff --git a/hphp/runtime/vm/translator/hopt/irtranslator.cpp b/hphp/runtime/vm/translator/hopt/irtranslator.cpp index 2be1bf108..6cf3114a6 100644 --- a/hphp/runtime/vm/translator/hopt/irtranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/irtranslator.cpp @@ -1108,7 +1108,9 @@ TranslatorX64::irTranslateFCallBuiltin(const Tracelet& t, HHIR_EMIT(FCallBuiltin, numArgs, numNonDefault, funcId); } -static bool shouldIRInline(const Func* func, const Tracelet& callee) { +static bool shouldIRInline(const Func* curFunc, + const Func* func, + const Tracelet& callee) { if (!RuntimeOption::EvalHHIREnableGenTimeInlining) { return false; } @@ -1243,6 +1245,15 @@ static bool shouldIRInline(const Func* func, const Tracelet& callee) { } } + /* + * Continuation allocation functions that take no arguments. + */ + resetCursor(); + if (current == OpCreateCont && cursor->imm[0].u_IVA == 0) { + next(); + if (atRet()) return accept("zero-arg continuation creator"); + } + /* * Anything that just puts a value on the stack with no inputs, and * then returns it, after possibly doing some comparison with @@ -1281,7 +1292,7 @@ TranslatorX64::irTranslateFCall(const Tracelet& t, */ if (i.calleeTrace) { if (!m_hhbcTrans->isInlining() && - shouldIRInline(i.funcd, *i.calleeTrace)) { + shouldIRInline(curFunc(), i.funcd, *i.calleeTrace)) { m_hhbcTrans->beginInlining(numArgs, i.funcd, returnBcOffset); static const bool shapeStats = Stats::enabledAny() && getenv("HHVM_STATS_INLINESHAPE"); diff --git a/hphp/runtime/vm/translator/hopt/linearscan.cpp b/hphp/runtime/vm/translator/hopt/linearscan.cpp index 7de3567f7..eed847552 100644 --- a/hphp/runtime/vm/translator/hopt/linearscan.cpp +++ b/hphp/runtime/vm/translator/hopt/linearscan.cpp @@ -668,6 +668,8 @@ void LinearScan::computePreColoringHint() { m_preColoringHint.add(inst->getSrc(arg.srcIdx), 0, reg++); m_preColoringHint.add(inst->getSrc(arg.srcIdx), 1, reg++); break; + case Immed: + break; } } return; diff --git a/hphp/runtime/vm/translator/hopt/nativecalls.cpp b/hphp/runtime/vm/translator/hopt/nativecalls.cpp index 14805eecf..b30838b18 100644 --- a/hphp/runtime/vm/translator/hopt/nativecalls.cpp +++ b/hphp/runtime/vm/translator/hopt/nativecalls.cpp @@ -41,18 +41,22 @@ static const DestType DNone = DestType::None; * * Opcode * The opcode that uses the call + * * Func * A value describing the function to call: * (TCA) - Raw function pointer * {FSSA, idx} - Use a const TCA from inst->getSrc(idx) + * * Dest * DSSA - The helper returns a single-register value * DTV - The helper returns a TypedValue in two registers * DNone - The helper does not return a value + * * SyncPoint * SNone - The helper does not need a sync point * SSync - The helper needs a normal sync point * SSyncAdj1 - The helper needs a sync point that skips top of stack on unwind + * * Args * A list of tuples describing the arguments to pass to the helper * {SSA, idx} - Pass the value in inst->getSrc(idx) @@ -61,6 +65,8 @@ static const DestType DNone = DestType::None; * {VecKeyS, idx} - Like TV, but Str values are passed as a raw * StringData*, in a single register * {VecKeyIS, idx} - Like VecKeyS, including Int + * {Immed} - There's no precoloring to do here, an internal immediate is + * used in cgFoo for this function */ static CallMap s_callMap({ /* Opcode, Func, Dest, SyncPoint, Args */ @@ -167,6 +173,8 @@ static CallMap s_callMap({ /* Continuation support helpers */ {CreateCont, {FSSA, 0}, DSSA, SNone, {{SSA, 1}, {SSA, 2}, {SSA, 3}, {SSA, 4}}}, + {InlineCreateCont, nullptr, DSSA, SSync, + {{Immed}, {Immed}, {SSA, 0}}}, {FillContLocals, (TCA)&VMExecutionContext::fillContinuationVars, DNone, SNone, {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}}}, diff --git a/hphp/runtime/vm/translator/hopt/nativecalls.h b/hphp/runtime/vm/translator/hopt/nativecalls.h index dd0146c6e..d5bb95d2d 100644 --- a/hphp/runtime/vm/translator/hopt/nativecalls.h +++ b/hphp/runtime/vm/translator/hopt/nativecalls.h @@ -51,6 +51,7 @@ enum ArgType : unsigned { TV, VecKeyS, VecKeyIS, + Immed, }; struct Arg { ArgType type; diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index 79375008a..ab85c5d8c 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -166,6 +166,8 @@ SSATmp* Simplifier::simplify(IRInstruction* inst) { case SpillStack: return simplifySpillStack(inst); case Call: return simplifyCall(inst); + case ExitOnVarEnv: return simplifyExitOnVarEnv(inst); + default: return nullptr; } @@ -275,6 +277,16 @@ SSATmp* Simplifier::simplifyCall(IRInstruction* inst) { return nullptr; } +// We never inline functions that could have a VarEnv, so an +// ExitOnVarEnv that has a frame based on DefInlineFP can be removed. +SSATmp* Simplifier::simplifyExitOnVarEnv(IRInstruction* inst) { + auto const frameInst = inst->getSrc(0)->inst(); + if (frameInst->op() == DefInlineFP) { + inst->convertToNop(); + } + return nullptr; +} + SSATmp* Simplifier::simplifyLdCtx(IRInstruction* inst) { const Func* func = inst->getSrc(1)->getValFunc(); if (func->isStatic()) { diff --git a/hphp/runtime/vm/translator/hopt/simplifier.h b/hphp/runtime/vm/translator/hopt/simplifier.h index 457a47abd..e1814b87d 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.h +++ b/hphp/runtime/vm/translator/hopt/simplifier.h @@ -93,14 +93,13 @@ private: SSATmp* simplifyGetCtxFwdCall(IRInstruction* inst); SSATmp* simplifySpillStack(IRInstruction* inst); SSATmp* simplifyCall(IRInstruction* inst); - -private: SSATmp* genDefInt(int64_t val); SSATmp* genDefDbl(double val); SSATmp* genDefBool(bool val); SSATmp* simplifyCmp(Opcode opName, SSATmp* src1, SSATmp* src2); SSATmp* simplifyCondJmp(IRInstruction*); SSATmp* simplifyQueryJmp(IRInstruction*); + SSATmp* simplifyExitOnVarEnv(IRInstruction*); private: TraceBuilder* const m_tb; diff --git a/hphp/system/class_map.cpp b/hphp/system/class_map.cpp index bde2778a7..e7e8f7e75 100644 --- a/hphp/system/class_map.cpp +++ b/hphp/system/class_map.cpp @@ -24389,12 +24389,8 @@ const char *g_class_map[] = { "/**\n * ( excerpt from http://php.net/manual/en/class.continuation.php )\n *\n *\n */", "iterator", "awaitable", NULL, (const char *)0x10006040, "__construct", "", (const char*)0, (const char*)0, - "/**\n * ( excerpt from http://php.net/manual/en/continuation.construct.php )\n *\n *\n * @func int\n * @origFuncName\n * string\n * @obj mixed\n * @args map\n */", - (const char *)0x8 /* KindOfNull */, (const char *)0x2000, "func", "", (const char *)0xa /* KindOfInt64 */, "", (const char *)0, "", (const char *)0, NULL, - (const char *)0x2000, "origFuncName", "", (const char *)0x14 /* KindOfString */, "", (const char *)0, "", (const char *)0, NULL, - (const char *)0x2000, "obj", "", (const char *)0xffffffff /* KindOfUnknown: $t: Variant */, "N;", (const char *)2, "null", (const char *)4, NULL, - (const char *)0x2000, "args", "", (const char *)0x20 /* KindOfArray */, "N;", (const char *)2, "null", (const char *)4, NULL, - NULL, + "/**\n * ( excerpt from http://php.net/manual/en/continuation.construct.php )\n *\n *\n */", + (const char *)0x8 /* KindOfNull */, NULL, NULL, NULL, (const char *)0x10006040, "update", "", (const char*)0, (const char*)0, diff --git a/hphp/test/quick/inline_generator_body.php b/hphp/test/quick/inline_generator_body.php index 9487adbd3..e97d76b5f 100644 --- a/hphp/test/quick/inline_generator_body.php +++ b/hphp/test/quick/inline_generator_body.php @@ -10,6 +10,13 @@ class CGetM { function getX() { return $this->x; } + + public function genVarious() { + $local = $this->getX(); + yield "a"; + yield $local; + yield "c"; + } } function foo() { @@ -19,6 +26,16 @@ function foo() { yield "\n"; } -foreach (foo() as $x) { - echo $x; +function main() { + foreach (foo() as $x) { + echo $x; + } + + $blah = new CGetM; + foreach ($blah->genVarious() as $x) { + echo $x; + } } + +main(); +echo "\n"; diff --git a/hphp/test/quick/inline_generator_body.php.expect b/hphp/test/quick/inline_generator_body.php.expect index bf7e89904..b5d4ccad0 100644 --- a/hphp/test/quick/inline_generator_body.php.expect +++ b/hphp/test/quick/inline_generator_body.php.expect @@ -1 +1,2 @@ asdasd +aasdasdc