From 5d09e8a87c08cc30c0ddb6a1eea9d160923f3a15 Mon Sep 17 00:00:00 2001 From: Mirek Klimos Date: Wed, 26 Jun 2013 11:42:12 -0700 Subject: [PATCH] CreateCont opcode cleanup - removing FillContLocals instruction FillContLocals was used only when num of locals > kMaxInlineContLocals; it doesn't hurt performance to copy them always within hhir. --- hphp/doc/ir.specification | 9 +--- hphp/runtime/vm/jit/codegen.cpp | 1 - hphp/runtime/vm/jit/hhbctranslator.cpp | 64 +++++++++++--------------- hphp/runtime/vm/jit/ir.h | 4 -- hphp/runtime/vm/jit/nativecalls.cpp | 3 -- hphp/runtime/vm/jit/translator.h | 1 - 6 files changed, 29 insertions(+), 53 deletions(-) diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 845b79d43..4f61a2ca0 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -808,7 +808,7 @@ D:FuncCtx = LdClsMethodFCache S0:ConstStr S1:ConstStr names of the callee's class and method, respectively. S2 is the current context. This instruction loads the corresponding entry in the MethodFCache if needed. In case the given method is not found, - control is transferred to label L. The current frame pointer is + control is transferred to label L. The current frame pointer is passed in S3. D:Ctx = GetCtxFwdCall S0:Ctx S1:Func @@ -1509,13 +1509,6 @@ D:Obj = CreateContMeth S0:ConstFunc S1:ConstFunc S2:Ctx S1 - the generator function S2 - the m_this/m_cls field of the current frame pointer -FillContLocals S0:FramePtr S1:ConstFunc S2:ConstFunc S3:Obj - - Fill the locals for the continuation object S3. Copies the values - for the locals from the frame S0 to their heap location in the - continuation object. S1 is the "original" func, and S2 is the - generator function. - ContEnter S0:FramePtr S1:TCA S2:ConstInt S3:FramePtr Enters a generator body. S0 is the ActRec embedded in the Continuation diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index 132d483a6..22e265180 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/hphp/runtime/vm/jit/codegen.cpp @@ -399,7 +399,6 @@ CALL_OPCODE(ConvCellToStr); CALL_OPCODE(CreateContFunc) CALL_OPCODE(CreateContMeth) -CALL_OPCODE(FillContLocals) CALL_OPCODE(NewArray) CALL_OPCODE(NewTuple) CALL_OPCODE(AllocObj) diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp index 95e8d9ff7..19246ad49 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.cpp +++ b/hphp/runtime/vm/jit/hhbctranslator.cpp @@ -1008,25 +1008,21 @@ void HhbcTranslator::emitCIterFree(uint32_t iterId) { typedef std::map ContParamMap; /* - * mapContParams determines if every named local in origFunc has a - * corresponding named local in genFunc. If this step succeeds and + * mapContParams builds a mapping between named locals in origFunc and + * corresponding named locals in genFunc. If this step succeeds and * there's no VarEnv at runtime, the continuation's variables can be * filled completely inline in the TC (assuming there aren't too * many). */ static -bool mapContParams(ContParamMap& map, +void mapContParams(ContParamMap& map, const Func* origFunc, const Func* genFunc) { const StringData* const* varNames = origFunc->localNames(); for (Id i = 0; i < origFunc->numNamedLocals(); ++i) { Id id = genFunc->lookupVarId(varNames[i]); - if (id != kInvalidId) { - map[i] = id; - } else { - return false; - } + assert(id != kInvalidId); + map[i] = id; } - return true; } void HhbcTranslator::emitCreateCont(Id funNameStrId) { @@ -1051,34 +1047,30 @@ void HhbcTranslator::emitCreateCont(Id funNameStrId) { ); ContParamMap params; - if (origLocals <= Translator::kMaxInlineContLocals && - mapContParams(params, origFunc, genFunc)) { - static auto const thisStr = StringData::GetStaticString("this"); - Id thisId = kInvalidId; - const bool fillThis = origFunc->isMethod() && - !origFunc->isStatic() && - ((thisId = genFunc->lookupVarId(thisStr)) != kInvalidId) && - (origFunc->lookupVarId(thisStr) == kInvalidId); + mapContParams(params, origFunc, genFunc); - SSATmp* contAR = gen( - LdRaw, Type::PtrToGen, cont, cns(RawMemSlot::ContARPtr)); - for (int i = 0; i < origLocals; ++i) { - // We must generate an AssertLoc because we don't have tracelet - // guards on the object type in these outer generator functions. - gen(AssertLoc, Type::Gen, LocalId(i), m_tb->fp()); - // Copy the value of the local to the cont object and set the - // local to uninit so that we don't need to change refcounts. - gen(StMem, contAR, cns(-cellsToBytes(params[i] + 1)), ldLoc(i)); - gen(StLoc, LocalId(i), m_tb->fp(), m_tb->genDefUninit()); - } - if (fillThis) { - assert(thisId != kInvalidId); - auto const thisObj = gen(IncRef, gen(LdThis, m_tb->fp())); - gen(StMem, contAR, cns(-cellsToBytes(thisId + 1)), thisObj); - } - } else { - gen(FillContLocals, m_tb->fp(), cns(origFunc), - cns(genFunc), cont); + static auto const thisStr = StringData::GetStaticString("this"); + Id thisId = kInvalidId; + const bool fillThis = origFunc->isMethod() && + !origFunc->isStatic() && + ((thisId = genFunc->lookupVarId(thisStr)) != kInvalidId) && + (origFunc->lookupVarId(thisStr) == kInvalidId); + + SSATmp* contAR = gen( + LdRaw, Type::PtrToGen, cont, cns(RawMemSlot::ContARPtr)); + for (int i = 0; i < origLocals; ++i) { + // We must generate an AssertLoc because we don't have tracelet + // guards on the object type in these outer generator functions. + gen(AssertLoc, Type::Gen, LocalId(i), m_tb->fp()); + // Copy the value of the local to the cont object and set the + // local to uninit so that we don't need to change refcounts. + gen(StMem, contAR, cns(-cellsToBytes(params[i] + 1)), ldLoc(i)); + gen(StLoc, LocalId(i), m_tb->fp(), m_tb->genDefUninit()); + } + if (fillThis) { + assert(thisId != kInvalidId); + auto const thisObj = gen(IncRef, gen(LdThis, m_tb->fp())); + gen(StMem, contAR, cns(-cellsToBytes(thisId + 1)), thisObj); } push(cont); diff --git a/hphp/runtime/vm/jit/ir.h b/hphp/runtime/vm/jit/ir.h index f56fd5ebe..a498293be 100644 --- a/hphp/runtime/vm/jit/ir.h +++ b/hphp/runtime/vm/jit/ir.h @@ -472,10 +472,6 @@ O(Spill, DofS(0), SUnk, Mem) \ O(Reload, DofS(0), SUnk, Mem) \ O(CreateContFunc, D(Obj), C(Func) C(Func), E|N|PRc) \ O(CreateContMeth, D(Obj), C(Func) C(Func) S(Ctx), E|N|PRc) \ -O(FillContLocals, ND, S(FramePtr) \ - C(Func) \ - C(Func) \ - S(Obj), E|N|Mem) \ O(ContEnter, ND, S(FramePtr) \ S(TCA) C(Int) S(FramePtr), E|Mem) \ O(ContPreNext, ND, S(Obj), E|Mem) \ diff --git a/hphp/runtime/vm/jit/nativecalls.cpp b/hphp/runtime/vm/jit/nativecalls.cpp index 9cf467ca9..7cb65d33c 100644 --- a/hphp/runtime/vm/jit/nativecalls.cpp +++ b/hphp/runtime/vm/jit/nativecalls.cpp @@ -177,9 +177,6 @@ static CallMap s_callMap({ {{SSA, 0}, {SSA, 1}}}, {CreateContMeth, (TCA)&VMExecutionContext::createContMeth, DSSA, SNone, {{SSA, 0}, {SSA, 1}, {SSA, 2}}}, - {FillContLocals, (TCA)&VMExecutionContext::fillContinuationVars, - DNone, SNone, - {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}}}, /* VectorTranslator helpers */ {BaseG, {FSSA, 0}, DSSA, SSync, {{TV, 1}, {SSA, 2}}}, diff --git a/hphp/runtime/vm/jit/translator.h b/hphp/runtime/vm/jit/translator.h index e39823166..5450535e1 100644 --- a/hphp/runtime/vm/jit/translator.h +++ b/hphp/runtime/vm/jit/translator.h @@ -680,7 +680,6 @@ public: // kMaxInlineReturnDecRefs is the maximum ref-counted locals to // generate an inline return for. static const int kMaxInlineReturnDecRefs = 1; - static const int kMaxInlineContLocals = 10; private: friend struct TraceletContext;