Kill InlineCreateCont

Remove unnecessary optimization that can be achieved in a more general
way and simplify continuation creation code.

VMExecutionContext::createContinuationHelper was renamed to
VMExecutionContext::createCont{Func,Meth}. The createContFunc no longer
takes this/class arguments, the createContMeth transfers them in one
Type::Ctx pointer that is used natively by ActRec's m_this. Since the
whole logic of this function is to set this single field, the logic is
now simpler.

Interpreter: iopCreateCont() just passes the m_this field of the parent
ActRec.

Translator: CreateCont opcode loads m_this field using LdCtx opcode.
This opcode optimizes into LdThis, which optimizes into
DefInlineFP->SpillFrame->object and allows frame to be eliminated, a
case previously covered by InlineCreateCont.

This diff uncovered a bug in trace builder, where LdCtx in static
methods could be optimized into LdThis, if the method was called thru
object. Fixed.
Esse commit está contido em:
Jan Oravec
2013-06-09 19:17:39 -07:00
commit de sgolemon
commit 8491515257
11 arquivos alterados com 73 adições e 161 exclusões
+10 -13
Ver Arquivo
@@ -1470,23 +1470,20 @@ 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.
D:Obj = CreateCont S0:TCA S1:FramePtr S2:ConstBool S3:ConstFunc S4:ConstFunc
D:Obj = CreateContFunc S0:ConstFunc S1:ConstFunc
Create a continuation object. Arguments:
Create a continuation object from function. 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
S0 - the "original" function
S1 - the generator function
D:Obj = InlineCreateCont<origFunc,genFunc> S0:{Obj,Null}
D:Obj = CreateContMeth S0:ConstFunc S1:ConstFunc S2:Ctx
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.
Create a continuation object from method. Arguments:
S0 - the "original" method
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
+5 -10
Ver Arquivo
@@ -430,16 +430,11 @@ public:
void requestExit();
static void getElem(TypedValue* base, TypedValue* key, TypedValue* dest);
template<bool isMethod>
static c_Continuation* createContinuationHelper(
const Func* origFunc,
const Func* genFunc,
ObjectData* thisPtr,
Class* frameStaticCls);
template<bool isMethod>
static c_Continuation* createContinuation(ActRec* fp,
const Func* origFunc,
const Func* genFunc);
static c_Continuation* createContFunc(const Func* origFunc,
const Func* genFunc);
static c_Continuation* createContMeth(const Func* origFunc,
const Func* genFunc,
void* objOrCls);
static c_Continuation* fillContinuationVars(
ActRec* fp, const Func* origFunc, const Func* genFunc,
c_Continuation* cont);
+26 -47
Ver Arquivo
@@ -6981,12 +6981,8 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCreateCl(PC& pc) {
m_stack.pushObject(cl2);
}
template<bool isMethod>
c_Continuation*
VMExecutionContext::createContinuationHelper(const Func* origFunc,
const Func* genFunc,
ObjectData* thisPtr,
Class* frameStaticCls) {
static inline c_Continuation* createCont(const Func* origFunc,
const Func* genFunc) {
auto const cont = c_Continuation::alloc(
origFunc,
genFunc->numLocals(),
@@ -6999,22 +6995,6 @@ VMExecutionContext::createContinuationHelper(const Func* origFunc,
// 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) {
if (origFunc->isClosureBody()) {
genFunc = genFunc->cloneAndSetClass(origFunc->cls());
}
if (thisPtr) {
ar->setThis(thisPtr);
thisPtr->incRefCount();
} else {
ar->setClass(frameStaticCls);
}
} else {
ar->setThis(nullptr);
}
ar->m_func = genFunc;
ar->initNumArgs(1);
ar->setVarEnv(nullptr);
@@ -7030,19 +7010,29 @@ VMExecutionContext::createContinuationHelper(const Func* origFunc,
return cont;
}
template<bool isMethod>
c_Continuation*
VMExecutionContext::createContinuation(ActRec* fp,
const Func* origFunc,
const Func* genFunc) {
ObjectData* const thisPtr = fp->hasThis() ? fp->getThis() : nullptr;
VMExecutionContext::createContFunc(const Func* origFunc,
const Func* genFunc) {
auto cont = createCont(origFunc, genFunc);
cont->actRec()->setThis(nullptr);
return cont;
}
return createContinuationHelper<isMethod>(
origFunc,
genFunc,
thisPtr,
frameStaticClass(fp)
);
c_Continuation*
VMExecutionContext::createContMeth(const Func* origFunc,
const Func* genFunc,
void* objOrCls) {
if (origFunc->isClosureBody()) {
genFunc = genFunc->cloneAndSetClass(origFunc->cls());
}
auto cont = createCont(origFunc, genFunc);
auto ar = cont->actRec();
ar->setThisOrClass(objOrCls);
if (ar->hasThis()) {
ar->getThis()->incRefCount();
}
return cont;
}
static inline void setContVar(const Func* genFunc,
@@ -7107,16 +7097,6 @@ VMExecutionContext::fillContinuationVars(ActRec* fp,
return cont;
}
// Explicitly instantiate for hhbctranslator.o and codegen.o
template c_Continuation* VMExecutionContext::createContinuation<true>(
ActRec*, const Func*, const Func*);
template c_Continuation* VMExecutionContext::createContinuation<false>(
ActRec*, const Func*, const Func*);
template c_Continuation* VMExecutionContext::createContinuationHelper<true>(
const Func*, const Func*, ObjectData*, Class*);
template c_Continuation* VMExecutionContext::createContinuationHelper<false>(
const Func*, const Func*, ObjectData*, Class*);
inline void OPTBLD_INLINE VMExecutionContext::iopCreateCont(PC& pc) {
NEXT();
DECODE_LITSTR(genName);
@@ -7125,10 +7105,9 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCreateCont(PC& pc) {
const Func* genFunc = origFunc->getGeneratorBody(genName);
assert(genFunc != nullptr);
bool isMethod = origFunc->isMethod();
c_Continuation* cont = isMethod ?
createContinuation<true>(m_fp, origFunc, genFunc) :
createContinuation<false>(m_fp, origFunc, genFunc);
c_Continuation* cont = origFunc->isMethod()
? createContMeth(origFunc, genFunc, m_fp->getThisOrClass())
: createContFunc(origFunc, genFunc);
fillContinuationVars(m_fp, origFunc, genFunc, cont);
+9
Ver Arquivo
@@ -315,6 +315,15 @@ struct ActRec {
return uintptr_t(p) & 1 ? (Class*)(uintptr_t(p)&~1LL) : nullptr;
}
void setThisOrClass(void* objOrCls) {
m_this = (ObjectData*)objOrCls;
assert(hasThis() || hasClass());
}
void* getThisOrClass() const {
return m_this;
}
/**
* To conserve space, we use unions for pairs of mutually exclusive
* fields (fields that are not used at the same time). We use unions
+2 -26
Ver Arquivo
@@ -395,7 +395,8 @@ CALL_OPCODE(ConvIntToStr);
CALL_OPCODE(ConvObjToStr);
CALL_OPCODE(ConvCellToStr);
CALL_OPCODE(CreateCont)
CALL_OPCODE(CreateContFunc)
CALL_OPCODE(CreateContMeth)
CALL_OPCODE(FillContLocals)
CALL_OPCODE(NewArray)
CALL_OPCODE(NewTuple)
@@ -3485,31 +3486,6 @@ void CodeGenerator::cgAllocObjFast(IRInstruction* inst) {
}
}
void CodeGenerator::cgInlineCreateCont(IRInstruction* inst) {
auto const& data = *inst->extra<InlineCreateCont>();
auto const helper = data.origFunc->isMethod()
? &VMExecutionContext::createContinuationHelper<true>
: &VMExecutionContext::createContinuationHelper<false>;
cgCallHelper(
m_as,
reinterpret_cast<TCA>(helper),
inst->dst(),
kSyncPoint,
ArgGroup(m_regs)
.immPtr(data.origFunc)
.immPtr(data.genFunc)
.ssa(inst->src(0))
// 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->src(0)->isA(Type::Obj));
}
}
void CodeGenerator::cgCallArray(IRInstruction* inst) {
Offset pc = inst->extra<CallArray>()->pc;
Offset after = inst->extra<CallArray>()->after;
-38
Ver Arquivo
@@ -401,22 +401,6 @@ void optimizeActRecs(IRTrace* trace, DceState& state, IRFactory* factory,
forEachInst(trace, [&](IRInstruction* inst) {
switch (inst->op()) {
case CreateCont:
{
auto const frameInst = inst->src(1)->inst();
if (frameInst->op() == DefInlineFP) {
auto const spInst = frameInst->src(0)->inst();
if (spInst->op() == SpillFrame &&
spInst->src(3)->type().subtypeOfAny(Type::Obj, Type::Null)) {
FTRACE(5, "CreateCont ({}): weak use of frame {}\n",
inst->id(),
frameInst->id());
state[frameInst].incWeakUse();
}
}
}
break;
// We don't need to generate stores to a frame if it can be
// eliminated.
case StLoc:
@@ -460,28 +444,6 @@ void optimizeActRecs(IRTrace* trace, DceState& state, IRFactory* factory,
*/
forEachInst(trace, [&](IRInstruction* inst) {
switch (inst->op()) {
case CreateCont:
{
auto const fp = inst->src(1);
if (state[fp->inst()].isDead()) {
FTRACE(5, "CreateCont ({}) -> InlineCreateCont\n", inst->id());
CreateContData data;
data.origFunc = inst->src(2)->getValFunc();
data.genFunc = inst->src(3)->getValFunc();
assert(fp->inst()->src(0)->inst()->op() == SpillFrame);
auto const thisPtr = fp->inst()->src(0)->inst()->src(3);
factory->replace(
inst,
InlineCreateCont,
data,
thisPtr
);
}
}
break;
case StLoc: case InlineReturn:
{
auto const fp = inst->src(0);
-6
Ver Arquivo
@@ -190,11 +190,6 @@ private:
uintptr_t m_dataBits;
};
struct CreateContData : IRExtraData {
const Func* origFunc;
const Func* genFunc;
};
/*
* Information for the REQ_BIND_JMPCC stubs we create when a tracelet
* ends with conditional jumps.
@@ -349,7 +344,6 @@ X(DefInlineFP, DefInlineFPData);
X(ReqBindJmp, BCOffset);
X(ReqBindJmpNoIR, BCOffset);
X(ReqRetranslateNoIR, BCOffset);
X(InlineCreateCont, CreateContData);
X(CallArray, CallArrayData);
X(LdClsCns, ClsCnsName);
X(LookupClsCns, ClsCnsName);
+12 -11
Ver Arquivo
@@ -1004,17 +1004,18 @@ void HhbcTranslator::emitCreateCont(Id funNameStrId) {
auto const genFunc = origFunc->getGeneratorBody(genName);
auto const origLocals = origFunc->numLocals();
auto const cont = gen(
CreateCont,
cns(
origFunc->isMethod() ?
(TCA)&VMExecutionContext::createContinuation<true> :
(TCA)&VMExecutionContext::createContinuation<false>
),
m_tb->fp(),
cns(origFunc),
cns(genFunc)
);
auto const cont = origFunc->isMethod()
? gen(
CreateContMeth,
cns(origFunc),
cns(genFunc),
gen(LdCtx, m_tb->fp(), cns(curFunc()))
)
: gen(
CreateContFunc,
cns(origFunc),
cns(genFunc)
);
ContParamMap params;
if (origLocals <= Translator::kMaxInlineContLocals &&
+2 -5
Ver Arquivo
@@ -434,11 +434,8 @@ O(InterpOneCF, ND, S(FramePtr) S(StkPtr) \
C(Int), T|E|N|Mem|Refs|Er) \
O(Spill, DofS(0), SUnk, Mem) \
O(Reload, DofS(0), SUnk, Mem) \
O(CreateCont, D(Obj), C(TCA) \
S(FramePtr) \
C(Func) \
C(Func), E|N|Mem|PRc) \
O(InlineCreateCont, D(Obj), S(Obj,Null), E|N|PRc) \
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) \
+4 -4
Ver Arquivo
@@ -175,10 +175,10 @@ static CallMap s_callMap({
{{SSA, 0}, {SSA, 1}, {SSA, 2}}},
/* Continuation support helpers */
{CreateCont, {FSSA, 0}, DSSA, SNone,
{{SSA, 1}, {SSA, 2}, {SSA, 3}}},
{InlineCreateCont, nullptr, DSSA, SSync,
{{Immed}, {Immed}, {SSA, 0}}},
{CreateContFunc, (TCA)&VMExecutionContext::createContFunc, DSSA, SNone,
{{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}}},
+3 -1
Ver Arquivo
@@ -129,10 +129,12 @@ void TraceBuilder::trackDefInlineFP(IRInstruction* inst) {
*
* We set m_thisIsAvailable to true on any object method, because we
* just don't inline calls to object methods with a null $this.
*
* Static methods probably broken #2490252.
*/
m_fpValue = calleeFP;
m_spValue = calleeSP;
m_thisIsAvailable = target->cls() != nullptr;
m_thisIsAvailable = target->cls() != nullptr && !target->isStatic();
m_curFunc = cns(target);
/*