diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 01011c728..8d2ea6887 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -1516,16 +1516,6 @@ ContEnter S0:FramePtr S1:TCA S2:ConstInt S3:FramePtr object. S1 is the address to jump to. S2 is the bytecode offset in the caller to return to when the generator body yields. S3 is the current frame. -UnlinkContVarEnv S0:FramePtr - - Part of PackCont. Unlinks the VarEnv associated with the - continuation frame S0 from the execution context, if it has one. - -LinkContVarEnv S0:FramePtr - - Part of UnpackCont. Links the VarEnv associated with the - continuation into the execution context, if it has one. - ContRaiseCheck S0:Obj -> L Checks whethre the continuation object S0 has raised an exception, diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index db8791b59..156458cb9 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -437,8 +437,6 @@ public: static c_Continuation* fillContinuationVars( ActRec* fp, const Func* origFunc, const Func* genFunc, c_Continuation* cont); - static void unpackContVarEnvLinkage(ActRec* fp); - static void packContVarEnvLinkage(ActRec* fp); void pushLocalsAndIterators(const HPHP::Func* f, int nparams = 0); void enqueueSharedVar(SharedVariant* var); @@ -580,7 +578,6 @@ public: static void PrintTCCallerInfo(); VarEnv* m_globalVarEnv; - VarEnv* m_topVarEnv; HPHP::RenamedFuncDict m_renamedFuncs; EvaledFilesMap m_evaledFiles; diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index e419d84b4..b419a2f25 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -219,8 +219,8 @@ static inline Class* frameStaticClass(ActRec* fp) { VarEnv::VarEnv() : m_depth(0) , m_malloced(false) + , m_global(false) , m_cfp(0) - , m_previous(0) , m_nvTable(boost::in_place( RuntimeOption::EvalVMInitialGlobalTableSize)) { @@ -237,6 +237,7 @@ VarEnv::VarEnv(ActRec* fp, ExtraArgs* eArgs) : m_extraArgs(eArgs) , m_depth(1) , m_malloced(false) + , m_global(false) , m_cfp(fp) { const Func* func = fp->m_func; @@ -260,9 +261,6 @@ VarEnv::~VarEnv() { this, isGlobalScope() ? "global scope" : "local scope"); assert(m_restoreLocations.empty()); - if (g_vmContext->m_topVarEnv == this) { - g_vmContext->m_topVarEnv = m_previous; - } if (!isGlobalScope()) { if (LIKELY(!m_malloced)) { @@ -280,50 +278,34 @@ VarEnv::~VarEnv() { } } -VarEnv* VarEnv::createLazyAttach(ActRec* fp, - bool skipInsert /* = false */) { - const Func* func = fp->m_func; - const size_t numNames = func->numNamedLocals(); - ExtraArgs* eArgs = fp->getExtraArgs(); - const size_t neededSz = sizeof(VarEnv) + - sizeof(TypedValue*) * numNames; +size_t VarEnv::getObjectSz(ActRec* fp) { + return sizeof(VarEnv) + sizeof(TypedValue*) * fp->m_func->numNamedLocals(); +} - TRACE(3, "Creating lazily attached VarEnv\n"); +VarEnv* VarEnv::createLocalOnStack(ActRec* fp) { + auto& va = varenv_arena(); + va.beginFrame(); + void* mem = va.alloc(getObjectSz(fp)); + VarEnv* ret = new (mem) VarEnv(fp, fp->getExtraArgs()); + TRACE(3, "Creating lazily attached VarEnv %p on stack\n", mem); + return ret; +} - if (LIKELY(!skipInsert)) { - auto& va = varenv_arena(); - va.beginFrame(); - void* mem = va.alloc(neededSz); - VarEnv* ret = new (mem) VarEnv(fp, eArgs); - TRACE(3, "Creating lazily attached VarEnv %p\n", mem); - ret->setPrevious(g_vmContext->m_topVarEnv); - g_vmContext->m_topVarEnv = ret; - return ret; - } - - /* - * For skipInsert == true, we're adding a VarEnv in the middle of - * the chain, which means we can't use the stack allocation. - * - * The caller must immediately setPrevious, so don't bother setting - * it to an invalid pointer except in a debug build. - */ - void* mem = malloc(neededSz); - VarEnv* ret = new (mem) VarEnv(fp, eArgs); +VarEnv* VarEnv::createLocalOnHeap(ActRec* fp) { + void* mem = malloc(getObjectSz(fp)); + VarEnv* ret = new (mem) VarEnv(fp, fp->getExtraArgs()); + TRACE(3, "Creating lazily attached VarEnv %p on heap\n", mem); ret->m_malloced = true; - if (debug) { - ret->setPrevious((VarEnv*)-1); - } return ret; } VarEnv* VarEnv::createGlobal() { assert(!g_vmContext->m_globalVarEnv); - assert(!g_vmContext->m_topVarEnv); VarEnv* ret = new (request_arena()) VarEnv(); TRACE(3, "Creating VarEnv %p [global scope]\n", ret); - g_vmContext->m_globalVarEnv = g_vmContext->m_topVarEnv = ret; + ret->m_global = true; + g_vmContext->m_globalVarEnv = ret; return ret; } @@ -1303,31 +1285,15 @@ void VMExecutionContext::addRenameableFunctions(ArrayData* arr) { VarEnv* VMExecutionContext::getVarEnv() { VMRegAnchor _; - VarEnv* builtinVarEnv = nullptr; ActRec* fp = getFP(); if (UNLIKELY(!fp)) return NULL; if (fp->skipFrame()) { - if (fp->hasVarEnv()) { - builtinVarEnv = fp->getVarEnv(); - } fp = getPrevVMState(fp); } if (!fp) return nullptr; assert(!fp->hasInvName()); if (!fp->hasVarEnv()) { - if (builtinVarEnv) { - // If the builtin function has its own VarEnv, we temporarily - // remove it from the list before making a VarEnv for the calling - // function to satisfy various asserts - assert(builtinVarEnv == m_topVarEnv); - m_topVarEnv = m_topVarEnv->previous(); - } - fp->m_varEnv = VarEnv::createLazyAttach(fp); - if (builtinVarEnv) { - // Put the builtin function's VarEnv back in the list - builtinVarEnv->setPrevious(fp->m_varEnv); - m_topVarEnv = builtinVarEnv; - } + fp->setVarEnv(VarEnv::createLocalOnStack(fp)); } return fp->m_varEnv; } @@ -2467,7 +2433,7 @@ bool VMExecutionContext::evalUnit(Unit* unit, PC& pc, int funcType) { assert(isReturnHelper(ar->m_savedRip)); pushLocalsAndIterators(func); if (!m_fp->hasVarEnv()) { - m_fp->m_varEnv = VarEnv::createLazyAttach(m_fp); + m_fp->setVarEnv(VarEnv::createLocalOnStack(m_fp)); } ar->m_varEnv = m_fp->m_varEnv; ar->m_varEnv->attach(ar); @@ -2638,16 +2604,7 @@ void VMExecutionContext::evalPHPDebugger(TypedValue* retval, StringData *code, ActRec *fp = getFP(); ActRec *cfpSave = nullptr; if (fp) { - VarEnv* vit = nullptr; for (; frame > 0; --frame) { - if (fp->hasVarEnv()) { - if (!vit) { - vit = m_topVarEnv; - } else if (vit != fp->m_varEnv) { - vit = vit->previous(); - } - assert(vit == fp->m_varEnv); - } ActRec* prevFp = getPrevVMState(fp); if (!prevFp) { // To be safe in case we failed to get prevFp. This would mean we've @@ -2658,15 +2615,7 @@ void VMExecutionContext::evalPHPDebugger(TypedValue* retval, StringData *code, fp = prevFp; } if (!fp->hasVarEnv()) { - if (!vit) { - fp->m_varEnv = VarEnv::createLazyAttach(fp); - } else { - const bool skipInsert = true; - fp->m_varEnv = VarEnv::createLazyAttach(fp, skipInsert); - // Slide it in front of the VarEnv most recently above it. - fp->m_varEnv->setPrevious(vit->previous()); - vit->setPrevious(fp->m_varEnv); - } + fp->setVarEnv(VarEnv::createLocalOnHeap(fp)); } varEnv = fp->m_varEnv; cfpSave = varEnv->getCfp(); @@ -2748,7 +2697,6 @@ void VMExecutionContext::enterDebuggerDummyEnv() { if (!s_debuggerDummy) { s_debuggerDummy = compile_string("", 7); } - VarEnv* varEnv = m_topVarEnv; if (!getFP()) { assert(m_stack.count() == 0); ActRec* ar = m_stack.allocA(); @@ -2762,13 +2710,12 @@ void VMExecutionContext::enterDebuggerDummyEnv() { m_pc = s_debuggerDummy->entry(); m_firstAR = ar; } - m_fp->setVarEnv(varEnv); - varEnv->attach(m_fp); + m_fp->setVarEnv(m_globalVarEnv); + m_globalVarEnv->attach(m_fp); } void VMExecutionContext::exitDebuggerDummyEnv() { - assert(m_topVarEnv); - assert(m_globalVarEnv == m_topVarEnv); + assert(m_globalVarEnv); m_globalVarEnv->detach(getFP()); } @@ -2845,7 +2792,7 @@ static inline void lookupd_var(ActRec* fp, } else { assert(!fp->hasInvName()); if (!fp->hasVarEnv()) { - fp->m_varEnv = VarEnv::createLazyAttach(fp); + fp->setVarEnv(VarEnv::createLocalOnStack(fp)); } val = fp->m_varEnv->lookup(name); if (val == nullptr) { @@ -6752,7 +6699,7 @@ static inline void setContVar(const Func* genFunc, // We pass skipInsert to this VarEnv because it's going to exist // independent of the chain; i.e. we can't stack-allocate it. We link it // into the chain in UnpackCont, and take it out in PackCont. - contFP->setVarEnv(VarEnv::createLazyAttach(contFP, true)); + contFP->setVarEnv(VarEnv::createLocalOnHeap(contFP)); } contFP->getVarEnv()->setWithRef(name, src); } @@ -6868,21 +6815,10 @@ void VMExecutionContext::iopContExit(PC& pc) { m_fp = prevFp; } -void VMExecutionContext::unpackContVarEnvLinkage(ActRec* fp) { - // This is called from the TC, and is assumed not to reenter. - if (fp->hasVarEnv()) { - VarEnv*& topVE = g_vmContext->m_topVarEnv; - fp->getVarEnv()->setPrevious(topVE); - topVE = fp->getVarEnv(); - } -} - inline void OPTBLD_INLINE VMExecutionContext::iopUnpackCont(PC& pc) { NEXT(); c_Continuation* cont = frame_continuation(m_fp); - unpackContVarEnvLinkage(m_fp); - // Return the received value TypedValue* recv_to = m_stack.allocTV(); TypedValue* recv_fr = cont->m_received.asTypedValue(); @@ -6895,18 +6831,11 @@ inline void OPTBLD_INLINE VMExecutionContext::iopUnpackCont(PC& pc) { label->m_data.num = cont->m_label; } -void VMExecutionContext::packContVarEnvLinkage(ActRec* fp) { - if (fp->hasVarEnv()) { - g_vmContext->m_topVarEnv = fp->getVarEnv()->previous(); - } -} - inline void OPTBLD_INLINE VMExecutionContext::iopPackCont(PC& pc) { NEXT(); DECODE_IVA(label); c_Continuation* cont = frame_continuation(m_fp); - packContVarEnvLinkage(m_fp); cont->c_Continuation::t_update(label, tvAsCVarRef(m_stack.topTV())); m_stack.popTV(); } @@ -7375,9 +7304,8 @@ void VMExecutionContext::requestExit() { EnvConstants::requestExit(); if (m_globalVarEnv) { - assert(m_topVarEnv = m_globalVarEnv); VarEnv::destroy(m_globalVarEnv); - m_globalVarEnv = m_topVarEnv = 0; + m_globalVarEnv = 0; } varenv_arena().~VarEnvArena(); diff --git a/hphp/runtime/vm/bytecode.h b/hphp/runtime/vm/bytecode.h index f610b04c3..94620aefa 100644 --- a/hphp/runtime/vm/bytecode.h +++ b/hphp/runtime/vm/bytecode.h @@ -127,8 +127,8 @@ class VarEnv { ExtraArgs* m_extraArgs; uint16_t m_depth; bool m_malloced; + bool m_global; ActRec* m_cfp; - VarEnv* m_previous; // TODO remove vector (#1099580). Note: trying changing this to a // TinyVector<> for now increased icache misses, but maybe will be // feasable later (see D511561). @@ -153,27 +153,16 @@ class VarEnv { * used when we need a variable environment for some caller frame * (because we're about to attach a callee frame using attach()) but * don't actually have one. - * - * `skipInsert' means not to insert the new VarEnv on the front of - * g_vmContext->m_topVarEnv. In this case the caller must - * immediately call setPrevious() as appropriate---this is used to - * support the debugger, creating VarEnvs for frames in the middle - * of the ActRec chain. */ - static VarEnv* createLazyAttach(ActRec* fp, bool skipInsert = false); + static VarEnv* createLocalOnStack(ActRec* fp); + static VarEnv* createLocalOnHeap(ActRec* fp); // Allocate a global VarEnv. Initially not attached to any frame. static VarEnv* createGlobal(); static void destroy(VarEnv*); - /* - * Walk the VarEnv chain. Returns null when we're out of variable - * environments. You can change the chain with setPrevious (see - * evalPHPDebugger). - */ - VarEnv* previous() { return m_previous; } - void setPrevious(VarEnv* p) { m_previous = p; } + static size_t getObjectSz(ActRec* fp); void attach(ActRec* fp); void detach(ActRec* fp); @@ -192,7 +181,7 @@ class VarEnv { // Used for save/store m_cfp for debugger void setCfp(ActRec* fp) { m_cfp = fp; } ActRec* getCfp() const { return m_cfp; } - bool isGlobalScope() const { return !m_previous; } + bool isGlobalScope() const { return m_global; } // Access to wrapped ExtraArgs, if we have one. TypedValue* getExtraArg(unsigned argInd) const; diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index 5027e3d86..f2eaa9f8a 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/hphp/runtime/vm/jit/codegen.cpp @@ -5182,18 +5182,6 @@ void CodeGenerator::emitContVarEnvHelperCall(SSATmp* fp, TCA helper) { }); } -void CodeGenerator::cgUnlinkContVarEnv(IRInstruction* inst) { - emitContVarEnvHelperCall( - inst->src(0), - (TCA)VMExecutionContext::packContVarEnvLinkage); -} - -void CodeGenerator::cgLinkContVarEnv(IRInstruction* inst) { - emitContVarEnvHelperCall( - inst->src(0), - (TCA)VMExecutionContext::unpackContVarEnvLinkage); -} - void CodeGenerator::cgContPreNext(IRInstruction* inst) { auto contReg = m_regs[inst->src(0)].reg(); diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp index 0fcf22e98..4f8dc3755 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.cpp +++ b/hphp/runtime/vm/jit/hhbctranslator.cpp @@ -1114,7 +1114,6 @@ void HhbcTranslator::emitContExit() { } void HhbcTranslator::emitUnpackCont() { - gen(LinkContVarEnv, m_tb->fp()); gen(AssertLoc, Type::Obj, LocalId(0), m_tb->fp()); auto const cont = ldLoc(0); @@ -1126,7 +1125,6 @@ void HhbcTranslator::emitUnpackCont() { } void HhbcTranslator::emitPackCont(int64_t labelId) { - gen(UnlinkContVarEnv, m_tb->fp()); gen(AssertLoc, Type::Obj, LocalId(0), m_tb->fp()); auto const cont = ldLoc(0); auto const newVal = popC(); diff --git a/hphp/runtime/vm/jit/ir.h b/hphp/runtime/vm/jit/ir.h index 338310b49..c21f75d36 100644 --- a/hphp/runtime/vm/jit/ir.h +++ b/hphp/runtime/vm/jit/ir.h @@ -455,8 +455,6 @@ O(FillContLocals, ND, S(FramePtr) \ S(Obj), E|N|Mem) \ O(ContEnter, ND, S(FramePtr) \ S(TCA) C(Int) S(FramePtr), E|Mem) \ -O(UnlinkContVarEnv, ND, S(FramePtr), E|N|Mem) \ -O(LinkContVarEnv, ND, S(FramePtr), E|N|Mem) \ O(ContPreNext, ND, S(Obj), E|Mem) \ O(ContStartedCheck, ND, S(Obj), E) \ O(IterInit, D(Bool), S(Arr,Obj) \