diff --git a/hphp/runtime/vm/func.h b/hphp/runtime/vm/func.h index 60ae00f6c..4d8bad869 100644 --- a/hphp/runtime/vm/func.h +++ b/hphp/runtime/vm/func.h @@ -243,6 +243,10 @@ struct Func { } bool isNoInjection() const { return bool(m_attrs & AttrNoInjection); } + bool mayHaveThis() const { + return isPseudoMain() || (isMethod() && !isStatic()); + } + HphpArray* getStaticLocals() const; void getFuncInfo(ClassInfo::MethodInfo* mi) const; diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp index 7d6d9ee7f..dd25a71a2 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.cpp +++ b/hphp/runtime/vm/jit/hhbctranslator.cpp @@ -2104,10 +2104,6 @@ void HhbcTranslator::emitFCallBuiltin(uint32_t numArgs, push(ret); } -static bool mayHaveThis(const Func* func) { - return func->isPseudoMain() || (func->isMethod() && !func->isStatic()); -} - void HhbcTranslator::emitRetFromInlined(Type type) { SSATmp* retVal = pop(type); @@ -2157,6 +2153,7 @@ SSATmp* HhbcTranslator::emitDecRefLocalsInline(SSATmp* retVal) { SSATmp* retValSrcLoc = nullptr; Opcode retValSrcOpc = Nop; // Nop flags the ref-count opt is impossible IRInstruction* retValSrcInstr = retVal->inst(); + const Func* curFunc = this->curFunc(); /* * In case retVal comes from a local, the logic below tweaks the code @@ -2173,7 +2170,7 @@ SSATmp* HhbcTranslator::emitDecRefLocalsInline(SSATmp* retVal) { } } - if (mayHaveThis(curFunc())) { + if (curFunc->mayHaveThis()) { if (retValSrcLoc && retValSrcOpc == LdThis) { gen(DecRef, retVal); } else { @@ -2189,7 +2186,7 @@ SSATmp* HhbcTranslator::emitDecRefLocalsInline(SSATmp* retVal) { */ int retValLocId = (!isInlining() && retValSrcLoc && retValSrcOpc == LdLoc) ? retValSrcLoc->inst()->extra()->locId : -1; - for (int id = curFunc()->numLocals() - 1; id >= 0; --id) { + for (int id = curFunc->numLocals() - 1; id >= 0; --id) { if (retValLocId == id) { gen(DecRef, retVal); continue; @@ -2223,7 +2220,7 @@ void HhbcTranslator::emitRet(Type type, bool freeInline) { gen(StRetVal, m_tb->fp(), useRet); sp = gen(RetAdjustStack, m_tb->fp()); } else { - if (mayHaveThis(curFunc)) { + if (curFunc->mayHaveThis()) { gen(DecRefThis, m_tb->fp()); } sp = gen(GenericRetDecRefs, m_tb->fp(), cns(curFunc->numLocals())); diff --git a/hphp/runtime/vm/jit/irtranslator.cpp b/hphp/runtime/vm/jit/irtranslator.cpp index 01c79e6aa..9335a7e66 100644 --- a/hphp/runtime/vm/jit/irtranslator.cpp +++ b/hphp/runtime/vm/jit/irtranslator.cpp @@ -1198,7 +1198,9 @@ bool shouldIRInline(const Func* curFunc, FTRACE(1, "CreateCont with {} args\n", func->numParams()); } next(); - if (atRet()) return accept("continuation creator"); + if (atRet()) { + return accept("continuation creator"); + } } /* diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp index a68ecb9f0..dbf918e1e 100644 --- a/hphp/runtime/vm/jit/translator.cpp +++ b/hphp/runtime/vm/jit/translator.cpp @@ -2964,19 +2964,17 @@ static bool checkTaintFuncs(StringData* name) { * Check whether the a given FCall should be analyzed for possible * inlining or not. */ -static bool shouldAnalyzeCallee(const NormalizedInstruction* fcall) { +static bool shouldAnalyzeCallee(const NormalizedInstruction* fcall, + const FPIEnt* fpi, + const Opcode pushOp) { auto const numArgs = fcall->imm[0].u_IVA; auto const target = fcall->funcd; - auto const fpi = curFunc()->findFPI(fcall->source.offset()); - auto const pushOp = curUnit()->getOpcode(fpi->m_fpushOff); if (!RuntimeOption::RepoAuthoritative) return false; - // Note: the IR assumes that $this is available in all inlined object - // methods, which will need to be updated when we support - // OpFPushClsMethod here. if (pushOp != OpFPushFuncD && pushOp != OpFPushObjMethodD - && pushOp != OpFPushCtorD && pushOp != OpFPushCtor) { + && pushOp != OpFPushCtorD && pushOp != OpFPushCtor + && pushOp != OpFPushClsMethodD) { FTRACE(1, "analyzeCallee: push op ({}) was not supported\n", opcodeToName(pushOp)); return false; @@ -3008,6 +3006,12 @@ static bool shouldAnalyzeCallee(const NormalizedInstruction* fcall) { return false; } + if (pushOp == OpFPushClsMethodD && target->mayHaveThis()) { + FTRACE(1, "analyzeCallee: not inlining static calls which may have a " + "this pointer\n"); + return false; + } + // Find the fpush and ensure it's in this tracelet---refuse to // inline if there are any calls in order to prepare arguments. for (auto* ni = fcall->prev; ni; ni = ni->prev) { @@ -3031,11 +3035,14 @@ extern bool shouldIRInline(const Func* curFunc, void Translator::analyzeCallee(TraceletContext& tas, Tracelet& parent, NormalizedInstruction* fcall) { - if (!shouldAnalyzeCallee(fcall)) return; + auto const callerFunc = curFunc(); + auto const fpi = callerFunc->findFPI(fcall->source.offset()); + auto const pushOp = curUnit()->getOpcode(fpi->m_fpushOff); + + if (!shouldAnalyzeCallee(fcall, fpi, pushOp)) return; auto const numArgs = fcall->imm[0].u_IVA; auto const target = fcall->funcd; - auto const callerFunc = curFunc(); /* * Prepare a map for all the known information about the argument diff --git a/hphp/test/slow/inlining/static_functions.php b/hphp/test/slow/inlining/static_functions.php new file mode 100644 index 000000000..ecb651189 --- /dev/null +++ b/hphp/test/slow/inlining/static_functions.php @@ -0,0 +1,29 @@ +next(); + var_dump($g->current()); + $g = Constants::gen2(); + $g->next(); + var_dump($g->current()); + $g = Constants::gen3("baz"); + $g->next(); + var_dump($g->current()); +} + +main(); diff --git a/hphp/test/slow/inlining/static_functions.php.expect b/hphp/test/slow/inlining/static_functions.php.expect new file mode 100644 index 000000000..8a0de6bf9 --- /dev/null +++ b/hphp/test/slow/inlining/static_functions.php.expect @@ -0,0 +1,3 @@ +string(3) "foo" +string(3) "bar" +string(3) "baz" diff --git a/hphp/test/slow/inlining/static_functions.php.opts b/hphp/test/slow/inlining/static_functions.php.opts new file mode 100644 index 000000000..4338db0ae --- /dev/null +++ b/hphp/test/slow/inlining/static_functions.php.opts @@ -0,0 +1 @@ +-vEval.JitEnableRenameFunction=0 -vEval.EnableHipHopSyntax=1 diff --git a/hphp/test/slow/ir_inlining/multi_static.php b/hphp/test/slow/ir_inlining/multi_static.php new file mode 100644 index 000000000..f20e6a8aa --- /dev/null +++ b/hphp/test/slow/ir_inlining/multi_static.php @@ -0,0 +1,50 @@ +next(); + yield $g->current(); + } + static public function genC($g) { + $g->next(); + yield $g->current(); + } + static public function genD($g) { + $g->next(); + yield $g->current(); + } + static public function genE($g) { + $g->next(); + yield $g->current(); + } + static public function genF($g) { + $g->next(); + yield $g->current(); + } + static public function genG($g) { + $g->next(); + yield $g->current(); + } + static public function genH($g) { + $g->next(); + yield $g->current(); + } +} + +function main() { + $g = Constants::genA(); + $g = Constants::genB($g); + $g = Constants::genC($g); + $g = Constants::genD($g); + $g = Constants::genE($g); + $g = Constants::genF($g); + $g = Constants::genG($g); + $g = Constants::genH($g); + $g->next(); + var_dump($g->current()); +} + +main(); diff --git a/hphp/test/slow/ir_inlining/multi_static.php.expect b/hphp/test/slow/ir_inlining/multi_static.php.expect new file mode 100644 index 000000000..cfae6e9a7 --- /dev/null +++ b/hphp/test/slow/ir_inlining/multi_static.php.expect @@ -0,0 +1 @@ +string(3) "foo" diff --git a/hphp/test/slow/ir_inlining/multi_static.php.opts b/hphp/test/slow/ir_inlining/multi_static.php.opts new file mode 100644 index 000000000..4338db0ae --- /dev/null +++ b/hphp/test/slow/ir_inlining/multi_static.php.opts @@ -0,0 +1 @@ +-vEval.JitEnableRenameFunction=0 -vEval.EnableHipHopSyntax=1 diff --git a/hphp/test/slow/ir_inlining/weird_static.php b/hphp/test/slow/ir_inlining/weird_static.php new file mode 100644 index 000000000..035c86470 --- /dev/null +++ b/hphp/test/slow/ir_inlining/weird_static.php @@ -0,0 +1,15 @@ +next(); + var_dump($g->current()); +} + +main(); diff --git a/hphp/test/slow/ir_inlining/weird_static.php.expect b/hphp/test/slow/ir_inlining/weird_static.php.expect new file mode 100644 index 000000000..cfae6e9a7 --- /dev/null +++ b/hphp/test/slow/ir_inlining/weird_static.php.expect @@ -0,0 +1 @@ +string(3) "foo" diff --git a/hphp/test/slow/ir_inlining/weird_static.php.opts b/hphp/test/slow/ir_inlining/weird_static.php.opts new file mode 100644 index 000000000..4338db0ae --- /dev/null +++ b/hphp/test/slow/ir_inlining/weird_static.php.opts @@ -0,0 +1 @@ +-vEval.JitEnableRenameFunction=0 -vEval.EnableHipHopSyntax=1