From bf3a854c842ddc29c1cb2371399432b364b9b4f3 Mon Sep 17 00:00:00 2001 From: Paul Bissonnette Date: Thu, 20 Aug 2015 15:33:36 -0700 Subject: [PATCH] Move inlining to translateRegion Summary: Moves logic for inlining into translateRegion and removes knowledge of inlined blocks from region-desc Reviewed By: @ottoni Differential Revision: D2330225 --- hphp/runtime/vm/hhbc.h | 20 +++ hphp/runtime/vm/jit/frame-state.cpp | 38 +++-- hphp/runtime/vm/jit/frame-state.h | 11 +- hphp/runtime/vm/jit/inlining-decider.cpp | 125 +++++---------- hphp/runtime/vm/jit/inlining-decider.h | 13 +- hphp/runtime/vm/jit/ir-builder.cpp | 20 +-- hphp/runtime/vm/jit/ir-builder.h | 15 +- hphp/runtime/vm/jit/irgen-control.cpp | 11 +- hphp/runtime/vm/jit/irgen-inlining.cpp | 19 ++- hphp/runtime/vm/jit/mc-generator.cpp | 5 +- hphp/runtime/vm/jit/prof-data.cpp | 2 +- hphp/runtime/vm/jit/region-method.cpp | 2 +- hphp/runtime/vm/jit/region-prune-arcs.cpp | 14 -- hphp/runtime/vm/jit/region-selection.cpp | 20 +-- hphp/runtime/vm/jit/region-selection.h | 23 +-- hphp/runtime/vm/jit/region-tracelet.cpp | 180 +++------------------ hphp/runtime/vm/jit/translate-region.cpp | 252 ++++++++++++++++++++---------- hphp/runtime/vm/jit/translate-region.h | 28 ++-- hphp/test/quick/exceptions_clone.php.ini | 1 + 19 files changed, 359 insertions(+), 440 deletions(-) create mode 100644 hphp/test/quick/exceptions_clone.php.ini diff --git a/hphp/runtime/vm/hhbc.h b/hphp/runtime/vm/hhbc.h index 1d6fb4a4920..e8c231ff57b 100644 --- a/hphp/runtime/vm/hhbc.h +++ b/hphp/runtime/vm/hhbc.h @@ -1088,6 +1088,22 @@ constexpr bool isFPush(Op opcode) { return opcode >= OpFPushFunc && opcode <= OpFPushCufSafe; } +constexpr bool isFPushCuf(Op opcode) { + return opcode >= OpFPushCufIter && opcode <= OpFPushCufSafe; +} + +constexpr bool isFPushClsMethod(Op opcode) { + return opcode >= OpFPushClsMethod && opcode <= OpFPushClsMethodD; +} + +constexpr bool isFPushCtor(Op opcode) { + return opcode == OpFPushCtor || opcode == OpFPushCtorD; +} + +constexpr bool isFPushFunc(Op opcode) { + return opcode >= OpFPushFunc && opcode <= OpFPushFuncU; +} + inline bool isFCallStar(Op opcode) { switch (opcode) { case Op::FCall: @@ -1123,6 +1139,10 @@ constexpr bool isRet(Op op) { return op == OpRetC || op == OpRetV; } +constexpr bool isReturnish(Op op) { + return isRet(op) || op == Op::NativeImpl; +} + constexpr bool isSwitch(Op op) { return op == OpSwitch || op == OpSSwitch; } diff --git a/hphp/runtime/vm/jit/frame-state.cpp b/hphp/runtime/vm/jit/frame-state.cpp index 6d279e58a78..c51a7453abb 100644 --- a/hphp/runtime/vm/jit/frame-state.cpp +++ b/hphp/runtime/vm/jit/frame-state.cpp @@ -143,13 +143,18 @@ bool merge_into(FrameState& dst, const FrameState& src) { always_assert(dstInfo.returnSP == srcInfo.returnSP); always_assert(dstInfo.returnSPOff == srcInfo.returnSPOff); + always_assert(isFPush(dstInfo.fpushOpc) && + dstInfo.fpushOpc == srcInfo.fpushOpc); - // If one of the merged edges was interp'ed forget the FPush kind - if (dstInfo.kind != srcInfo.kind) { - always_assert(dstInfo.kind == FPIInfo::SpillKind::FPushUnknown || - srcInfo.kind == FPIInfo::SpillKind::FPushUnknown); + // If one of the merged edges was interp'ed mark the result as interp'ed + if (!dstInfo.interp && srcInfo.interp) { + dstInfo.interp = true; + changed = true; + } - dstInfo.kind = FPIInfo::SpillKind::FPushUnknown; + // If one of the merged edges spans a call then mark them both as spanning + if (!dstInfo.spansCall && srcInfo.spansCall) { + dstInfo.spansCall = true; changed = true; } @@ -368,6 +373,9 @@ bool FrameStateMgr::update(const IRInstruction* inst) { if (!cur().fpiStack.empty()) { cur().fpiStack.pop_front(); } + for (auto& st : m_stack) { + for (auto& fpi : st.fpiStack) fpi.spansCall = true; + } } break; @@ -393,6 +401,9 @@ bool FrameStateMgr::update(const IRInstruction* inst) { if (!cur().fpiStack.empty()) { cur().fpiStack.pop_front(); } + for (auto& st : m_stack) { + for (auto& fpi : st.fpiStack) fpi.spansCall = true; + } } break; @@ -551,7 +562,7 @@ bool FrameStateMgr::update(const IRInstruction* inst) { case CufIterSpillFrame: spillFrameStack(inst->extra()->spOffset, - cur().spOffset, nullptr); + cur().spOffset, inst); break; case SpillFrame: spillFrameStack(inst->extra()->spOffset, @@ -567,7 +578,9 @@ bool FrameStateMgr::update(const IRInstruction* inst) { cur().fpiStack.push_front(FPIInfo { cur().spValue, cur().spOffset, nullptr, - FPIInfo::SpillKind::FPushUnknown }); + extra.opcode, + true /* interp */, + false /* spansCall */}); } else if (isFCallStar(extra.opcode) && !cur().fpiStack.empty()) { cur().fpiStack.pop_front(); } @@ -1193,15 +1206,12 @@ void FrameStateMgr::spillFrameStack(IRSPOffset offset, FPInvOffset retOffset, for (auto i = uint32_t{0}; i < kNumActRecCells; ++i) { setStackValue(offset + i, nullptr); } - auto const kind = - !inst ? FPIInfo::SpillKind::FPushUnknown - : !inst->src(2) ? FPIInfo::SpillKind::FPushFunc - : inst->extra()->fromFPushCtor ? FPIInfo::SpillKind::FPushCtor - : FPIInfo::SpillKind::FPushMethod; + auto const opc = inst->marker().sk().op(); - auto const ctx = inst ? inst->src(2) : nullptr; + auto const ctx = inst->op() == SpillFrame ? inst->src(2) : nullptr; cur().syncedSpLevel += kNumActRecCells; - cur().fpiStack.push_front(FPIInfo { cur().spValue, retOffset, ctx, kind }); + cur().fpiStack.push_front(FPIInfo { cur().spValue, retOffset, ctx, opc, + false /* interp */, false /* spans */ }); } void FrameStateMgr::refineStackType(IRSPOffset offset, diff --git a/hphp/runtime/vm/jit/frame-state.h b/hphp/runtime/vm/jit/frame-state.h index 2952d32d0fa..6fa5caa63cb 100644 --- a/hphp/runtime/vm/jit/frame-state.h +++ b/hphp/runtime/vm/jit/frame-state.h @@ -43,17 +43,12 @@ Type refinePredictedType(Type oldPrediction, Type newPrediction, Type proven); Type updatePredictedType(Type predictedType, Type provenType); struct FPIInfo { - enum class SpillKind { - FPushCtor, - FPushMethod, - FPushFunc, - FPushUnknown - }; - SSATmp* returnSP; FPInvOffset returnSPOff; // return's logical sp offset; stkptr might differ SSATmp* ctx; - SpillKind kind; + Op fpushOpc; // bytecode for FPush* + bool interp; + bool spansCall; }; struct EvalStack { diff --git a/hphp/runtime/vm/jit/inlining-decider.cpp b/hphp/runtime/vm/jit/inlining-decider.cpp index 66facedcbcd..8a3f745058d 100644 --- a/hphp/runtime/vm/jit/inlining-decider.cpp +++ b/hphp/runtime/vm/jit/inlining-decider.cpp @@ -22,10 +22,10 @@ #include "hphp/runtime/vm/bytecode.h" #include "hphp/runtime/vm/func.h" #include "hphp/runtime/vm/hhbc.h" -#include "hphp/runtime/vm/srckey.h" +#include "hphp/runtime/vm/jit/irgen.h" #include "hphp/runtime/vm/jit/normalized-instruction.h" #include "hphp/runtime/vm/jit/region-selection.h" -#include "hphp/runtime/vm/jit/irgen.h" +#include "hphp/runtime/vm/srckey.h" #include "hphp/util/trace.h" @@ -145,81 +145,6 @@ bool checkNumArgs(SrcKey callSK, const Func* callee) { return true; } -/* - * Check that the FPI region is suitable for inlining. - * - * We refuse to inline if the corresponding FPush is not found in the same - * region as the FCall, or if other calls are made between the two. - */ -bool checkFPIRegion(SrcKey callSK, const Func* callee, - const RegionDesc& region) { - assertx(callee); - - auto refuse = [&] (const char* why) { - return traceRefusal(callSK.func(), callee, why); - }; - - // Check that the FPush instruction is in the same region, and that our FCall - // is reachable from it. - // - // TODO(#4603302) Fix this once SrcKeys can appear multiple times in a region. - auto fpi = callSK.func()->findFPI(callSK.offset()); - const SrcKey pushSK { callSK.func(), - fpi->m_fpushOff, - callSK.resumed() }; - int pushBlock = -1; - - auto const& blocks = region.blocks(); - for (unsigned i = 0; i < blocks.size(); ++i) { - if (blocks[i]->contains(pushSK)) { - pushBlock = i; - break; - } - } - if (pushBlock == -1) { - return refuse("FPush* is not in the current region"); - } - - // Check that we have an acceptable FPush. - switch (pushSK.op()) { - case OpFPushClsMethodD: - if (callee->mayHaveThis()) { - return refuse("callee may have $this pointer"); - } - // fallthrough - case OpFPushFuncD: - case OpFPushObjMethodD: - case OpFPushCtorD: - case OpFPushCtor: - break; - - default: - return refuse(folly::format("unsupported push op {}", - opcodeToName(pushSK.op())).str().c_str()); - } - - // Calls invalidate all live SSATmps, so don't allow any in the FPI region. - for (unsigned i = pushBlock; i < blocks.size(); ++i) { - auto const& block = *blocks[i]; - - auto iterSK = (i == pushBlock ? pushSK.advanced() - : block.start()); - while (iterSK <= block.last()) { - // We're all set once we've hit the to-be-inlined FCall. - if (iterSK == callSK) return true; - - auto op = iterSK.op(); - - if (isFCallStar(op) || op == Op::FCallBuiltin) { - return refuse("FPI region contains another call"); - } - iterSK.advance(); - } - } - - not_reached(); -} - /////////////////////////////////////////////////////////////////////////////// } @@ -229,8 +154,7 @@ void InliningDecider::forbidInliningOf(const Func* callee) { forbiddenInlinees.insert(callee->fullName()); } -bool InliningDecider::canInlineAt(SrcKey callSK, const Func* callee, - const RegionDesc& region) const { +bool InliningDecider::canInlineAt(SrcKey callSK, const Func* callee) const { if (!callee || !RuntimeOption::EvalHHIREnableGenTimeInlining) { return false; } @@ -277,9 +201,7 @@ bool InliningDecider::canInlineAt(SrcKey callSK, const Func* callee, // TODO(#4238160): Inlining into pseudomain callsites is still buggy. if (callSK.func()->isPseudoMain()) return false; - if (!isCalleeInlinable(callSK, callee) || - !checkNumArgs(callSK, callee) || - !checkFPIRegion(callSK, callee, region)) { + if (!isCalleeInlinable(callSK, callee) || !checkNumArgs(callSK, callee)) { return false; } @@ -454,7 +376,7 @@ bool InliningDecider::shouldInline(const Func* callee, } // Count the returns. - if (isRet(op) || op == Op::NativeImpl) { + if (isReturnish(op)) { if (++numRets > 1) { return refuse("region has too many returns"); } @@ -503,5 +425,42 @@ void InliningDecider::registerEndInlining(const Func* callee) { m_stackDepth -= callee->maxStackCells(); } +RegionDescPtr selectCalleeRegion(const SrcKey& sk, + const Func* callee, + const IRGS& irgs) { + auto const op = reinterpret_cast(sk.pc()); + + auto const numArgs = getImm(op, 0).u_IVA; + auto const numParams = callee->numParams(); + + // Set up the RegionContext for the tracelet selector. + RegionContext ctx; + ctx.func = callee; + ctx.bcOffset = callee->getEntryForNumArgs(numArgs); + ctx.spOffset = FPInvOffset{safe_cast(callee->numSlotsInFrame())}; + ctx.resumed = false; + + for (int i = 0; i < numArgs; ++i) { + // DataTypeGeneric is used because we're just passing the locals into the + // callee. It's up to the callee to constrain further if needed. + auto type = irgen::publicTopType(irgs, BCSPOffset{i}); + + // If we don't have sufficient type information to inline the region + // return early + if (!(type <= TGen) && !(type <= TCls)) return nullptr; + uint32_t paramIdx = numArgs - 1 - i; + ctx.liveTypes.push_back({RegionDesc::Location::Local{paramIdx}, type}); + } + + for (unsigned i = numArgs; i < numParams; ++i) { + // These locals will be populated by DV init funclets but they'll start out + // as Uninit. + ctx.liveTypes.push_back({RegionDesc::Location::Local{i}, TUninit}); + } + + // Produce a tracelet for the callee. + return selectTracelet(ctx, false /* profiling */, true /* inlining */); +} + /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/vm/jit/inlining-decider.h b/hphp/runtime/vm/jit/inlining-decider.h index 4cf7adcbc4f..2172abaf5cd 100644 --- a/hphp/runtime/vm/jit/inlining-decider.h +++ b/hphp/runtime/vm/jit/inlining-decider.h @@ -17,6 +17,8 @@ #ifndef incl_HPHP_JIT_INLINING_H_ #define incl_HPHP_JIT_INLINING_H_ +#include "hphp/runtime/vm/jit/region-selection.h" + #include namespace HPHP { @@ -88,6 +90,7 @@ struct InliningDecider { */ bool disabled() const { return m_disabled; } int depth() const { return m_callDepth; } + bool inlining() const { return depth() != 0; } ///////////////////////////////////////////////////////////////////////////// // Core API. @@ -108,8 +111,7 @@ struct InliningDecider { * NOTE: Inlining will fail during translation if the FPush was interpreted. * It is up to the client to ensure that this is not the case. */ - bool canInlineAt(SrcKey callSK, const Func* callee, - const RegionDesc& region) const; + bool canInlineAt(SrcKey callSK, const Func* callee) const; /* * Check that `region' of `callee' can be inlined (possibly via other inlined @@ -156,6 +158,13 @@ private: std::vector m_costStack; }; +/* + * Select an inlining region for the call to `callee' at `sk'. + */ +RegionDescPtr selectCalleeRegion(const SrcKey& sk, + const Func* callee, + const IRGS& irgs); + /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/vm/jit/ir-builder.cpp b/hphp/runtime/vm/jit/ir-builder.cpp index 28b29522c4f..60efd25ca83 100644 --- a/hphp/runtime/vm/jit/ir-builder.cpp +++ b/hphp/runtime/vm/jit/ir-builder.cpp @@ -906,27 +906,27 @@ bool IRBuilder::startBlock(Block* block, bool hasUnprocessedPred) { return true; } -Block* IRBuilder::makeBlock(Offset offset) { - auto it = m_offsetToBlockMap.find(offset); - if (it == m_offsetToBlockMap.end()) { +Block* IRBuilder::makeBlock(SrcKey sk) { + auto it = m_skToBlockMap.find(sk); + if (it == m_skToBlockMap.end()) { auto const block = m_unit.defBlock(); - m_offsetToBlockMap.insert(std::make_pair(offset, block)); + m_skToBlockMap.emplace(sk, block); return block; } return it->second; } void IRBuilder::resetOffsetMapping() { - m_offsetToBlockMap.clear(); + m_skToBlockMap.clear(); } -bool IRBuilder::hasBlock(Offset offset) const { - return m_offsetToBlockMap.count(offset); +bool IRBuilder::hasBlock(SrcKey sk) const { + return m_skToBlockMap.count(sk); } -void IRBuilder::setBlock(Offset offset, Block* block) { - assertx(!hasBlock(offset)); - m_offsetToBlockMap[offset] = block; +void IRBuilder::setBlock(SrcKey sk, Block* block) { + assertx(!hasBlock(sk)); + m_skToBlockMap[sk] = block; } void IRBuilder::pushBlock(BCMarker marker, Block* b) { diff --git a/hphp/runtime/vm/jit/ir-builder.h b/hphp/runtime/vm/jit/ir-builder.h index face7419915..d2f9afe278e 100644 --- a/hphp/runtime/vm/jit/ir-builder.h +++ b/hphp/runtime/vm/jit/ir-builder.h @@ -183,7 +183,7 @@ public: /* * Create a new block corresponding to bytecode control flow. */ - Block* makeBlock(Offset offset); + Block* makeBlock(SrcKey sk); /* * Clear the map from bytecode offsets to Blocks. @@ -192,14 +192,14 @@ public: /* * Checks whether or not there's a block associated with the given - * bytecode offset. + * SrcKey offset. */ - bool hasBlock(Offset offset) const; + bool hasBlock(SrcKey sk) const; /* - * Set the block associated with the given offset in the offset->block map. + * Set the block associated with the given offset in the SrcKey->block map. */ - void setBlock(Offset offset, Block* block); + void setBlock(SrcKey sk, Block* block); /* * Get the block that we're currently emitting code to. @@ -335,10 +335,7 @@ private: GuardConstraints m_constraints; // Keep track of blocks created to support bytecode control flow. - // - // TODO(t3730559): Offset is used here since it's passed from - // emitJmp*, but SrcKey might be better in case of inlining. - jit::flat_map m_offsetToBlockMap; + jit::flat_map m_skToBlockMap; // Keeps the block to branch to (if any) in case a guard fails. // This holds nullptr if the guard failures should perform a service diff --git a/hphp/runtime/vm/jit/irgen-control.cpp b/hphp/runtime/vm/jit/irgen-control.cpp index af68013f3e2..b8162a8b0db 100644 --- a/hphp/runtime/vm/jit/irgen-control.cpp +++ b/hphp/runtime/vm/jit/irgen-control.cpp @@ -37,13 +37,14 @@ void surpriseCheck(IRGS& env, Offset relOffset) { * offset is in the current RegionDesc. */ Block* getBlock(IRGS& env, Offset offset) { + SrcKey sk(curSrcKey(env), offset); // If hasBlock returns true, then IRUnit already has a block for that offset // and makeBlock will just return it. This will be the proper successor // block set by setSuccIRBlocks. Otherwise, the given offset doesn't belong // to the region, so we just create an exit block. - if (!env.irb->hasBlock(offset)) return makeExit(env, offset); + if (!env.irb->hasBlock(sk)) return makeExit(env, offset); - return env.irb->makeBlock(offset); + return env.irb->makeBlock(sk); } ////////////////////////////////////////////////////////////////////// @@ -184,7 +185,8 @@ void emitSwitch(IRGS& env, // included in the region. auto const shouldLower = std::any_of(offsets.begin(), offsets.end(), [&](Offset o) { - return env.irb->hasBlock(bcOff(env) + o); + SrcKey sk(curSrcKey(env), bcOff(env) + o); + return env.irb->hasBlock(sk); }); if (shouldLower && profile.optimizing()) { auto const values = sortedSwitchProfile(profile, iv.size()); @@ -199,7 +201,8 @@ void emitSwitch(IRGS& env, // fully-generic JmpSwitchDest at the end if nothing matches. for (auto const& val : values) { auto targetOff = bcOff(env) + offsets[val.caseIdx]; - if (!env.irb->hasBlock(targetOff)) continue; + SrcKey sk(curSrcKey(env), targetOff); + if (!env.irb->hasBlock(sk)) continue; if (bounded && val.caseIdx == iv.size() - 2) { // If we haven't checked bounds yet and this is the "first non-zero" diff --git a/hphp/runtime/vm/jit/irgen-inlining.cpp b/hphp/runtime/vm/jit/irgen-inlining.cpp index 4fab618b56e..861394ba23a 100644 --- a/hphp/runtime/vm/jit/irgen-inlining.cpp +++ b/hphp/runtime/vm/jit/irgen-inlining.cpp @@ -79,15 +79,24 @@ bool beginInlining(IRGS& env, "FPI stack pointer and callee stack pointer didn't match in beginInlining" ); - always_assert(fpiStack.front().ctx || - fpiStack.front().kind == FPIInfo::SpillKind::FPushFunc); - always_assert(fpiStack.front().kind != FPIInfo::SpillKind::FPushUnknown); + auto const& info = fpiStack.front(); + always_assert(!isFPushCuf(info.fpushOpc) && !info.interp); + + auto ctx = [&] { + if (info.ctx || isFPushFunc(info.fpushOpc)) { + return info.ctx; + } + + constexpr int32_t adjust = offsetof(ActRec, m_r) - offsetof(ActRec, m_this); + IRSPOffset ctxOff{invSPOff(env) - info.returnSPOff - adjust}; + return gen(env, LdStk, TCtx, IRSPOffsetData{ctxOff}, sp(env)); + }(); DefInlineFPData data; data.target = target; data.retBCOff = returnBcOffset; - data.fromFPushCtor = fpiStack.front().kind == FPIInfo::SpillKind::FPushCtor; - data.ctx = fpiStack.front().ctx; + data.fromFPushCtor = isFPushCtor(info.fpushOpc); + data.ctx = ctx; data.retSPOff = prevSPOff; data.spOffset = offsetFromIRSP(env, BCSPOffset{0}); diff --git a/hphp/runtime/vm/jit/mc-generator.cpp b/hphp/runtime/vm/jit/mc-generator.cpp index 10e109d1cad..718850c58fe 100644 --- a/hphp/runtime/vm/jit/mc-generator.cpp +++ b/hphp/runtime/vm/jit/mc-generator.cpp @@ -1863,7 +1863,7 @@ MCGenerator::translateWork(const TranslArgs& args) { } auto result = TranslateResult::Retry; - auto regionInterps = RegionBlacklist{}; + TranslateRetryContext retry; initSpOffset = region ? region->entry()->initialSpOffset() : liveSpOff(); while (region && result == TranslateResult::Retry) { @@ -1882,8 +1882,7 @@ MCGenerator::translateWork(const TranslArgs& args) { assertCleanState(); maker.markStart(); - result = translateRegion(irgs, *region, regionInterps, args.flags, - pconds); + result = translateRegion(irgs, *region, retry, args.flags, pconds); hasLoop = RuntimeOption::EvalJitLoops && cfgHasLoop(irgs.unit); FTRACE(2, "translateRegion finished with result {}\n", show(result)); } catch (const std::exception& e) { diff --git a/hphp/runtime/vm/jit/prof-data.cpp b/hphp/runtime/vm/jit/prof-data.cpp index ee42757e290..f3697bb7e74 100644 --- a/hphp/runtime/vm/jit/prof-data.cpp +++ b/hphp/runtime/vm/jit/prof-data.cpp @@ -355,7 +355,7 @@ TransID ProfData::addTransProfile(const RegionDescPtr& region, assertx(region); DEBUG_ONLY size_t nBlocks = region->blocks().size(); - assertx(nBlocks == 1 || (nBlocks > 1 && region->entry()->inlinedCallee())); + assertx(nBlocks == 1); region->renumberBlock(region->entry()->id(), transId); for (auto& b : region->blocks()) b->setProfTransID(transId); region->blocks().back()->setPostConds(pconds); diff --git a/hphp/runtime/vm/jit/region-method.cpp b/hphp/runtime/vm/jit/region-method.cpp index 10ede0093c2..f52fe35059e 100644 --- a/hphp/runtime/vm/jit/region-method.cpp +++ b/hphp/runtime/vm/jit/region-method.cpp @@ -85,7 +85,7 @@ RegionDescPtr selectMethod(const RegionContext& context) { auto const start = unit->offsetOf(b->start); auto const length = numInstrs(b->start, b->end); SrcKey sk{context.func, start, context.resumed}; - auto const rblock = ret->addBlock(sk, length, spOffset, 0); + auto const rblock = ret->addBlock(sk, length, spOffset); blockMap[b] = rblock->id(); // flag SP offset as unknown for all but the first block spOffset = FPInvOffset::invalid(); diff --git a/hphp/runtime/vm/jit/region-prune-arcs.cpp b/hphp/runtime/vm/jit/region-prune-arcs.cpp index 0c7f0e53cff..579b9b68632 100644 --- a/hphp/runtime/vm/jit/region-prune-arcs.cpp +++ b/hphp/runtime/vm/jit/region-prune-arcs.cpp @@ -183,20 +183,6 @@ void region_prune_arcs(RegionDesc& region) { auto& binfo = blockInfos[rpoID]; FTRACE(4, "B{}\n", binfo.blockID); - /* - * This code currently assumes inlined functions were entirely contained - * within a single profiling translation, and will need updates if we - * inline bigger things in a way visible to region selection. - * - * Note: inlined blocks /may/ have postConditions, if they are the last - * blocks from profiling translations. Currently any locations referred to - * in postconditions for these blocks are for the outermost caller, so this - * code handles that correctly. - */ - if (region.block(binfo.blockID)->inlineLevel() != 0) { - assertx(region.block(binfo.blockID)->typePreConditions().empty()); - } - binfo.out = binfo.in; apply_transfer_function( binfo.out, diff --git a/hphp/runtime/vm/jit/region-selection.cpp b/hphp/runtime/vm/jit/region-selection.cpp index 28257236d07..0cc9afe82c1 100644 --- a/hphp/runtime/vm/jit/region-selection.cpp +++ b/hphp/runtime/vm/jit/region-selection.cpp @@ -136,11 +136,10 @@ SrcKey RegionDesc::lastSrcKey() const { RegionDesc::Block* RegionDesc::addBlock(SrcKey sk, int length, - FPInvOffset spOffset, - uint16_t inlineLevel) { + FPInvOffset spOffset) { m_blocks.push_back( std::make_shared(sk.func(), sk.resumed(), sk.offset(), length, - spOffset, inlineLevel)); + spOffset)); BlockPtr block = m_blocks.back(); m_data[block->id()] = BlockData(block); return block.get(); @@ -493,8 +492,7 @@ RegionDesc::Block::Block(const Func* func, bool resumed, Offset start, int length, - FPInvOffset initSpOff, - uint16_t inlineLevel) + FPInvOffset initSpOff) : m_id(s_nextId--) , m_func(func) , m_resumed(resumed) @@ -502,8 +500,6 @@ RegionDesc::Block::Block(const Func* func, , m_last(kInvalidOffset) , m_length(length) , m_initialSpOffset(initSpOff) - , m_inlinedCallee(nullptr) - , m_inlineLevel(inlineLevel) , m_profTransID(kInvalidTransID) { assertx(length >= 0); @@ -533,8 +529,6 @@ void RegionDesc::Block::addInstruction() { } void RegionDesc::Block::truncateAfter(SrcKey final) { - assert_not_implemented(!m_inlinedCallee); - auto skIter = start(); int newLen = -1; for (int i = 0; i < m_length; ++i, skIter.advance(unit())) { @@ -1083,7 +1077,6 @@ std::string show(const RegionDesc::Block& b) { b.start().resumed() ? "r" : "", " length ", b.length(), " initSpOff ", b.initialSpOffset().offset, - " inlineLevel ", b.inlineLevel(), " profTransID ", b.profTransID(), '\n', &ret @@ -1117,15 +1110,8 @@ std::string show(const RegionDesc::Block& b) { } if (topFunc) { const char* inlined = ""; - if (i == b.length() - 1 && b.inlinedCallee()) { - assertx(topFunc == b.inlinedCallee()); - inlined = " (call is inlined)"; - } knownFunc = folly::format(" (top func: {}{})", topFunc->fullName(), inlined).str(); - } else { - assertx((i < b.length() - 1 || !b.inlinedCallee()) && - "inlined FCall without a known funcd"); } std::string byRef; diff --git a/hphp/runtime/vm/jit/region-selection.h b/hphp/runtime/vm/jit/region-selection.h index 934f809635f..704bcbc2ff3 100644 --- a/hphp/runtime/vm/jit/region-selection.h +++ b/hphp/runtime/vm/jit/region-selection.h @@ -109,8 +109,7 @@ struct RegionDesc { */ SrcKey lastSrcKey() const; - Block* addBlock(SrcKey sk, int length, FPInvOffset spOffset, - uint16_t inlineLevel); + Block* addBlock(SrcKey sk, int length, FPInvOffset spOffset); void deleteBlock(BlockId bid); void renumberBlock(BlockId oldId, BlockId newId); void addArc(BlockId src, BlockId dst); @@ -312,7 +311,7 @@ struct RegionDesc::Block { using KnownFuncMap = boost::container::flat_map; explicit Block(const Func* func, bool resumed, Offset start, int length, - FPInvOffset initSpOff, uint16_t inlineLevel); + FPInvOffset initSpOff); Block& operator=(const Block&) = delete; @@ -331,7 +330,6 @@ struct RegionDesc::Block { bool empty() const { return length() == 0; } bool contains(SrcKey sk) const; FPInvOffset initialSpOffset() const { return m_initialSpOffset; } - uint16_t inlineLevel() const { return m_inlineLevel; } TransID profTransID() const { return m_profTransID; } void setID(BlockId id) { m_id = id; } @@ -339,19 +337,6 @@ struct RegionDesc::Block { void setInitialSpOffset(FPInvOffset sp) { m_initialSpOffset = sp; } /* - * Set and get whether or not this block ends with an inlined FCall. Inlined - * FCalls should not emit any code themselves, and they will be followed by - * one or more blocks from the callee. - */ - void setInlinedCallee(const Func* callee) { - assertx(callee); - m_inlinedCallee = callee; - } - const Func* inlinedCallee() const { - return m_inlinedCallee; - } - - /* * Increase the length of the Block by 1. */ void addInstruction(); @@ -428,8 +413,6 @@ private: Offset m_last; int m_length; FPInvOffset m_initialSpOffset; - const Func* m_inlinedCallee; - uint16_t m_inlineLevel; // 0 means the outer-most function TransID m_profTransID; TypedLocMap m_typePredictions; TypedLocMap m_typePreConditions; @@ -530,7 +513,7 @@ RegionDescPtr selectHotRegion(TransID transId, * whose shape will be analyzed by the InliningDecider. */ RegionDescPtr selectTracelet(const RegionContext& ctx, bool profiling, - bool allowInlining = true); + bool inlining = false); /* * Select the hottest trace beginning with triggerId. diff --git a/hphp/runtime/vm/jit/region-tracelet.cpp b/hphp/runtime/vm/jit/region-tracelet.cpp index 98f358c7553..b95dbbc1a79 100644 --- a/hphp/runtime/vm/jit/region-tracelet.cpp +++ b/hphp/runtime/vm/jit/region-tracelet.cpp @@ -53,8 +53,8 @@ namespace { struct RegionFormer { RegionFormer(const RegionContext& ctx, InterpSet& interp, - InliningDecider& inl, - bool profiling); + bool profiling, + bool inlining); RegionDescPtr go(); @@ -78,8 +78,8 @@ private: // map until it's been computed. jit::hash_map m_irReachableBlocks; - InliningDecider& m_inl; const bool m_profiling; + const bool m_inlining; const Func* curFunc() const; const Unit* curUnit() const; @@ -90,7 +90,6 @@ private: void addInstruction(); bool consumeInput(int i, const InputInfo& ii); bool traceThroughJmp(); - bool tryInline(uint32_t& instrSize); void recordDependencies(); void truncateLiterals(); bool irBlockReachable(Block* block); @@ -98,22 +97,22 @@ private: RegionFormer::RegionFormer(const RegionContext& ctx, InterpSet& interp, - InliningDecider& inl, - bool profiling) + bool profiling, + bool inlining) : m_ctx(ctx) , m_interp(interp) , m_sk(ctx.func, ctx.bcOffset, ctx.resumed) , m_startSk(m_sk) , m_region(std::make_shared()) - , m_curBlock(m_region->addBlock(m_sk, 0, ctx.spOffset, 0)) + , m_curBlock(m_region->addBlock(m_sk, 0, ctx.spOffset)) , m_blockFinished(false) // TODO(#5703534): this is using a different TransContext than actual // translation will use. , m_irgs(TransContext{kInvalidTransID, m_sk, ctx.spOffset}, TransFlags{0}) , m_arStates(1) , m_numJmps(0) - , m_inl(inl) , m_profiling(profiling) + , m_inlining(inlining) {} const Func* RegionFormer::curFunc() const { @@ -195,32 +194,6 @@ RegionDescPtr RegionFormer::go() { m_inst.interp = m_interp.count(m_sk); - uint32_t calleeInstrSize; - if (tryInline(calleeInstrSize)) { - // If m_inst is an FCall and the callee is suitable for inlining, we can - // translate the callee and potentially use its return type to extend the - // tracelet. - - auto callee = m_inst.funcd; - FTRACE(1, "\nselectTracelet starting inlined call from {} to " - "{} with stack:\n{}\n", curFunc()->fullName()->data(), - callee->fullName()->data(), show(m_irgs)); - auto returnSk = m_inst.nextSk(); - auto returnFuncOff = returnSk.offset() - curFunc()->base(); - - m_arStates.back().pop(); - m_arStates.emplace_back(); - m_curBlock->setInlinedCallee(callee); - irgen::beginInlining(m_irgs, m_inst.imm[0].u_IVA, callee, returnFuncOff); - - m_sk = irgen::curSrcKey(m_irgs); - m_blockFinished = true; - m_pendingInlinedInstrs += calleeInstrSize; - continue; - } - - auto const inlineReturn = irgen::isInlining(m_irgs) && - (isRet(m_inst.op()) || m_inst.op() == OpNativeImpl); try { translateInstr(m_irgs, m_inst, true /* checkOuterTypeOnly */, firstInst); } catch (const FailedIRGen& exn) { @@ -237,16 +210,6 @@ RegionDescPtr RegionFormer::go() { irgen::finishHHBC(m_irgs); - // If we just translated a return from an inlined call, grab the updated - // SrcKey from m_ht and clean up. - if (inlineReturn) { - m_inl.registerEndInlining(m_sk.func()); - m_sk = irgen::curSrcKey(m_irgs).advanced(curUnit()); - m_arStates.pop_back(); - m_blockFinished = true; - continue; - } - if (!instrAllowsFallThru(m_inst.op())) { FTRACE(1, "selectTracelet: tracelet broken after instruction with no " "fall-through {}\n", m_inst); @@ -282,25 +245,14 @@ RegionDescPtr RegionFormer::go() { if (isFCallStar(m_inst.op())) m_arStates.back().pop(); } - // If we failed while trying to inline, trigger retry without inlining. - if (m_region && !m_region->empty() && irgen::isInlining(m_irgs)) { - // We can recover from this situation just fine, but it's more often - // than not indicative of a real bug somewhere else in the system. - // TODO: 5515310 investigate whether legit bugs cause this. - FTRACE(1, "selectTracelet: Failed while inlining:\n{}\n{}", - show(*m_region), show(m_irgs.irb->unit())); - m_inl.disable(); - m_region.reset(); - } - if (m_region && !m_region->empty()) { // Make sure we end the region before trying to print the IRUnit. irgen::endRegion(m_irgs, m_sk); printUnit( kTraceletLevel, m_irgs.irb->unit(), - m_inl.depth() || m_inl.disabled() ? " after inlining tracelet formation " - : " after tracelet formation ", + m_inlining ? " after inlining tracelet formation " + : " after tracelet formation ", nullptr, m_irgs.irb->guards() ); @@ -395,8 +347,7 @@ void RegionFormer::addInstruction() { FTRACE(2, "selectTracelet adding new block at {} after:\n{}\n", showShort(m_sk), show(*m_curBlock)); always_assert(m_sk.func() == curFunc()); - auto newCurBlock = m_region->addBlock(m_sk, 0, curSpOffset(), - m_irgs.inlineLevel); + auto newCurBlock = m_region->addBlock(m_sk, 0, curSpOffset()); m_region->addArc(m_curBlock->id(), newCurBlock->id()); m_curBlock = newCurBlock; m_blockFinished = false; @@ -405,39 +356,35 @@ void RegionFormer::addInstruction() { FTRACE(2, "selectTracelet adding instruction {}\n", m_inst.toString()); m_curBlock->addInstruction(); m_numBCInstrs++; - if (irgen::isInlining(m_irgs)) m_pendingInlinedInstrs--; } bool RegionFormer::traceThroughJmp() { - // This flag means "if we are currently inlining, or we are generating a - // tracelet for inlining analysis". The latter is the case when inlining is - // disabled (because our caller wants to peek at our region without us - // inlining ourselves). - bool inlining = m_inl.depth() || m_inl.disabled(); - // We only trace through unconditional jumps and conditional jumps with const // inputs while inlining. if (!isUnconditionalJmp(m_inst.op()) && - !(inlining && isConditionalJmp(m_inst.op()) && + !(m_inlining && isConditionalJmp(m_inst.op()) && irgen::publicTopType(m_irgs, BCSPOffset{0}).hasConstVal())) { return false; } - // Normally we want to keep profiling translations to basic blocks, but if - // we're inlining we want to analyze as much of the callee as possible. - if (m_profiling && !inlining) return false; + // We want to keep profiling translations to basic blocks, inlining shouldn't + // happen in profiling translations + if (m_profiling) { + assert(!m_inlining); + return false; + } // Don't trace through too many jumps, unless we're inlining. We want to make // sure we don't break a tracelet in the middle of an inlined call; if the // inlined callee becomes too big that's caught in shouldIRInline. - if (m_numJmps == Translator::MaxJmpsTracedThrough && !inlining) { + if (m_numJmps == Translator::MaxJmpsTracedThrough && !m_inlining) { return false; } auto offset = m_inst.imm[0].u_BA; // Only trace through backwards jumps if it's a JmpNS and we're // inlining. This is to get DV funclets. - if (offset <= 0 && (m_inst.op() != OpJmpNS || !inlining)) { + if (offset <= 0 && (m_inst.op() != OpJmpNS || !m_inlining)) { return false; } @@ -462,83 +409,6 @@ bool RegionFormer::traceThroughJmp() { return true; } -bool RegionFormer::tryInline(uint32_t& instrSize) { - assertx(m_inst.source == m_sk); - assertx(m_inst.func() == curFunc()); - assertx(m_sk.resumed() == resumed()); - - instrSize = 0; - - if (!m_inl.canInlineAt(m_inst.source, m_inst.funcd, *m_region)) { - return false; - } - - auto refuse = [this](const std::string& str) { - FTRACE(2, "selectTracelet not inlining {}: {}\n", - m_inst.toString(), str); - return false; - }; - - auto callee = m_inst.funcd; - - auto const& fpiStack = m_irgs.irb->fpiStack(); - // Make sure the FPushOp wasn't interpreted. - if (fpiStack.empty()) { - return refuse("fpistack empty; fpush was in a different region"); - } - auto const& info = fpiStack.front(); - if (info.kind == FPIInfo::SpillKind::FPushUnknown) { - return refuse("couldn't find SpillFrame for FPushOp"); - } - - if (info.kind != FPIInfo::SpillKind::FPushFunc && !info.ctx) { - return refuse("Couldn't find context for SpillFrame"); - } - - auto numArgs = m_inst.imm[0].u_IVA; - auto numParams = callee->numParams(); - - // Set up the region context, mapping stack slots in the caller to locals in - // the callee. - RegionContext ctx; - ctx.func = callee; - ctx.bcOffset = callee->getEntryForNumArgs(numArgs); - ctx.spOffset = FPInvOffset{safe_cast(callee->numSlotsInFrame())}; - ctx.resumed = false; - for (int i = 0; i < numArgs; ++i) { - auto type = irgen::publicTopType(m_irgs, BCSPOffset{i}); - if (!type.subtypeOfAny(TCell, TBoxedInitCell)) { - return refuse(folly::sformat("argument {}'s type was too strange ({})", - i, type.toString())); - } - uint32_t paramIdx = numArgs - 1 - i; - ctx.liveTypes.push_back({RegionDesc::Location::Local{paramIdx}, type}); - } - - for (unsigned i = numArgs; i < numParams; ++i) { - // These locals will be populated by DV init funclets but they'll start - // out as Uninit. - ctx.liveTypes.push_back({RegionDesc::Location::Local{i}, TUninit}); - } - - FTRACE(1, "selectTracelet analyzing callee {} with context:\n{}", - callee->fullName()->data(), show(ctx)); - auto region = selectTracelet(ctx, m_profiling, false /* noinline */); - if (!region) { - return refuse("failed to select region in callee"); - } - - instrSize = region->instrSize(); - auto newInstrSize = instrSize + m_numBCInstrs + m_pendingInlinedInstrs; - if (newInstrSize > RuntimeOption::EvalJitMaxRegionInstrs) { - return refuse("new region would be too large"); - } - if (!m_inl.shouldInline(callee, *region)) { - return refuse("shouldIRInline failed"); - } - return true; -} - bool isLiteral(Op op) { switch (op) { case OpNull: @@ -746,18 +616,14 @@ void RegionFormer::recordDependencies() { } RegionDescPtr selectTracelet(const RegionContext& ctx, bool profiling, - bool allowInlining /* = true */) { + bool inlining /* = false */) { Timer _t(Timer::selectTracelet); InterpSet interp; RegionDescPtr region; uint32_t tries = 1; - InliningDecider inl(ctx.func); - if (!allowInlining) inl.disable(); - - while (!(region = RegionFormer(ctx, interp, inl, profiling).go())) { + while (!(region = RegionFormer(ctx, interp, profiling, inlining).go())) { ++tries; - inl.resetState(); } if (region->empty() || region->blocks().front()->length() == 0) { @@ -765,8 +631,8 @@ RegionDescPtr selectTracelet(const RegionContext& ctx, bool profiling, return RegionDescPtr { nullptr }; } - FTRACE(1, "selectTracelet returning, inlining {}, {} tries:\n{}\n", - allowInlining ? "allowed" : "disallowed", tries, show(*region)); + FTRACE(1, "selectTracelet returning, {}, {} tries:\n{}\n", + inlining ? "inlining" : "not inlining", tries, show(*region)); if (region->blocks().back()->length() == 0) { // If the final block is empty because it would've only contained // instructions producing literal values, kill it. diff --git a/hphp/runtime/vm/jit/translate-region.cpp b/hphp/runtime/vm/jit/translate-region.cpp index 943577cc75a..65199cc1385 100644 --- a/hphp/runtime/vm/jit/translate-region.cpp +++ b/hphp/runtime/vm/jit/translate-region.cpp @@ -57,8 +57,6 @@ BlockIdToIRBlockMap createBlockMap(IRGS& irgs, const RegionDesc& region) { for (unsigned i = 0; i < blocks.size(); i++) { auto const rBlock = blocks[i]; auto const id = rBlock->id(); - DEBUG_ONLY Offset bcOff = rBlock->start().offset(); - assertx(IMPLIES(i == 0, bcOff == irb.unit().bcOff())); // NB: This maps the region entry block to a new IR block, even though // we've already constructed an IR entry block. We'll make the IR entry @@ -68,7 +66,7 @@ BlockIdToIRBlockMap createBlockMap(IRGS& irgs, const RegionDesc& region) { ret[id] = iBlock; FTRACE(1, "createBlockMaps: RegionBlock {} => IRBlock {} (BC offset = {})\n", - id, iBlock->id(), bcOff); + id, iBlock->id(), rBlock->start().offset()); } return ret; @@ -84,15 +82,15 @@ void setIRBlock(IRGS& irgs, const BlockIdToIRBlockMap& blockIdToIRBlock) { auto& irb = *irgs.irb; auto rBlock = region.block(blockId); - Offset bcOffset = rBlock->start().offset(); + auto sk = rBlock->start(); auto iit = blockIdToIRBlock.find(blockId); assertx(iit != blockIdToIRBlock.end()); - assertx(!irb.hasBlock(bcOffset)); + assertx(!irb.hasBlock(sk)); FTRACE(3, " setIRBlock: blockId {}, offset {} => IR Block {}\n", - blockId, bcOffset, iit->second->id()); - irb.setBlock(bcOffset, iit->second); + blockId, sk.offset(), iit->second->id()); + irb.setBlock(sk, iit->second); } /* @@ -281,11 +279,9 @@ void emitPredictionsAndPreConditions(IRGS& irgs, } } irgen::ringbufferEntry(irgs, Trace::RBTypeTraceletBody, sk); - } - // In the entry block, hhbc-translator gets a chance to emit some code - // immediately after the initial checks on the first instruction. - if (block == region.entry()) { + // In the entry block, hhbc-translator gets a chance to emit some code + // immediately after the initial checks on the first instruction. switch (arch()) { case Arch::X64: irgen::prepareEntry(irgs); @@ -356,16 +352,6 @@ bool shouldTrySingletonInline(const RegionDesc& region, return false; } - auto nextOp = inst.nextSk().op(); - - // If the normal machinery is already inlining this function, don't - // do anything here. - if (instIdx == block->length() - 2 && - (nextOp == Op::FCall || nextOp == Op::FCallD) && - block->inlinedCallee()) { - return false; - } - return true; } @@ -533,13 +519,6 @@ folly::Optional nextReachableBlock( return folly::none; } -RegionDesc::BlockId singleSucc(const RegionDesc& region, - RegionDesc::BlockId bid) { - auto const& succs = region.succs(bid); - always_assert(succs.size() == 1); - return *succs.begin(); -} - /* * Returns whether or not block `bid' is in the retranslation chain * for `region's entry block. @@ -556,10 +535,62 @@ bool inEntryRetransChain(RegionDesc::BlockId bid, const RegionDesc& region) { not_reached(); } +/* + * If `psk' is not an FCall{,D} with inlinable `callee', return nullptr. + * + * Otherwise, select a region for `callee' if one is not already present in + * `retry'. Update `inl' and return the region if it's inlinable. + */ +RegionDescPtr getInlinableCalleeRegion(const ProfSrcKey& psk, + const Func* callee, + TranslateRetryContext& retry, + InliningDecider& inl, + const IRGS& irgs) { + if (psk.srcKey.op() != Op::FCall && + psk.srcKey.op() != Op::FCallD) { + return nullptr; + } + + if (!inl.canInlineAt(psk.srcKey, callee)) return nullptr; + + auto const& fpiStack = irgs.irb->fpiStack(); + // Make sure the FPushOp was in the region + if (fpiStack.empty()) { + return nullptr; + } + + // Make sure the FPushOp wasn't interpreted, based on an FPushCuf, or spanned + // another call + auto const info = fpiStack.front(); + if (isFPushCuf(info.fpushOpc) || info.interp || info.spansCall) { + return nullptr; + } + + // We can't inline FPushClsMethod when the callee may have a $this pointer + if (isFPushClsMethod(info.fpushOpc) && callee->mayHaveThis()) { + return nullptr; + } + + RegionDescPtr calleeRegion; + // Look up or select a region for `callee'. + if (retry.inlines.count(psk)) { + calleeRegion = retry.inlines[psk]; + } else { + calleeRegion = selectCalleeRegion(psk.srcKey, callee, irgs); + retry.inlines[psk] = calleeRegion; + } + if (!calleeRegion) return nullptr; + + // Return the callee region if it's inlinable and update `inl'. + return inl.shouldInline(callee, *calleeRegion) ? calleeRegion + : nullptr; +} + TranslateResult irGenRegion(IRGS& irgs, const RegionDesc& region, - RegionBlacklist& toInterp, - TransFlags trflags) { + TranslateRetryContext& retry, + TransFlags trflags, + InliningDecider& inl) { const Timer translateRegionTimer(Timer::translateRegion); FTRACE(1, "translateRegion starting with:\n{}\n", show(region)); @@ -575,16 +606,23 @@ TranslateResult irGenRegion(IRGS& irgs, // Create a map from region blocks to their corresponding initial IR blocks. auto blockIdToIRBlock = createBlockMap(irgs, region); - // Prepare to start translation of the first region block. - auto const entry = irb.unit().entry(); - irb.startBlock(entry, false /* hasUnprocPred */); - - // Make the IR entry block jump to the IR block we mapped the region entry - // block to (they are not the same!). - { - auto const irBlock = blockIdToIRBlock[region.entry()->id()]; - always_assert(irBlock != entry); - irgen::gen(irgs, Jmp, irBlock); + if (!inl.inlining()) { + // Prepare to start translation of the first region block. + auto const entry = irb.unit().entry(); + irb.startBlock(entry, false /* hasUnprocPred */); + + // Make the IR entry block jump to the IR block we mapped the region entry + // block to (they are not the same!). + { + auto const irBlock = blockIdToIRBlock[region.entry()->id()]; + always_assert(irBlock != entry); + irgen::gen(irgs, Jmp, irBlock); + } + } else { + // Set the first callee block as a successor to the FCall's block and + // "fallthrough" from the caller into the callee's first block. + setIRBlock(irgs, region.entry()->id(), region, blockIdToIRBlock); + irgen::endBlock(irgs, region.start().offset(), false); } RegionDesc::BlockIdSet processedBlocks; @@ -607,8 +645,8 @@ TranslateResult irGenRegion(IRGS& irgs, const Func* topFunc = nullptr; if (hasTransID(blockId)) irgs.profTransID = getTransID(blockId); - irgs.inlineLevel = block->inlineLevel(); - irgs.firstBcInst = inEntryRetransChain(blockId, region); + irgs.inlineLevel = inl.depth(); + irgs.firstBcInst = inEntryRetransChain(blockId, region) && !inl.inlining(); irgen::prepareForNextHHBC(irgs, nullptr, sk, false); // Prepare to start translating this region block. This loads the @@ -644,7 +682,7 @@ TranslateResult irGenRegion(IRGS& irgs, // the first instruction in the region, we check inner type eagerly, insert // `EndGuards` after the checks, and generate profiling code in profiling // translations. - auto const isEntry = block == region.entry(); + auto const isEntry = block == region.entry() && !inl.inlining(); auto const checkOuterTypeOnly = !isEntry || mcg->tx().mode() != TransKind::Profile; emitPredictionsAndPreConditions(irgs, region, block, isEntry); @@ -652,6 +690,7 @@ TranslateResult irGenRegion(IRGS& irgs, // Generate IR for each bytecode instruction in this block. for (unsigned i = 0; i < block->length(); ++i, sk.advance(block->unit())) { + ProfSrcKey psk { irgs.profTransID, sk }; auto const lastInstr = i == block->length() - 1; // Update bcOff here so any guards or assertions from metadata are @@ -665,34 +704,10 @@ TranslateResult irGenRegion(IRGS& irgs, // Create and initialize the instruction. NormalizedInstruction inst(sk, block->unit()); - bool toInterpInst = toInterp.count(ProfSrcKey{irgs.profTransID, sk}); + bool toInterpInst = retry.toInterp.count(psk); initNormalizedInstruction(inst, byRefs, irgs, region, blockId, topFunc, lastInstr, toInterpInst); - // If this block ends with an inlined FCall, we don't emit anything for - // the FCall and instead set up irgen for inlining. Blocks from - // the callee will be next in the region. - if (lastInstr && block->inlinedCallee()) { - always_assert(inst.op() == Op::FCall || inst.op() == Op::FCallD); - auto const* callee = block->inlinedCallee(); - FTRACE(1, "\nstarting inlined call from {} to {} with {} args " - "and stack:\n{}\n", - block->func()->fullName()->data(), - callee->fullName()->data(), - inst.imm[0].u_IVA, - show(irgs)); - auto returnSk = inst.nextSk(); - auto returnFuncOff = returnSk.offset() - block->func()->base(); - if (irgen::beginInlining(irgs, inst.imm[0].u_IVA, callee, - returnFuncOff)) { - // "Fallthrough" into the callee's first block - auto const calleeEntry = region.block(singleSucc(region, blockId)); - irgen::endBlock(irgs, calleeEntry->start().offset(), - inst.nextIsMerge); - } - continue; - } - // Singleton inlining optimization. if (RuntimeOption::EvalHHIRInlineSingletons && !lastInstr && shouldTrySingletonInline(region, block, inst, i, trflags) && @@ -711,6 +726,71 @@ TranslateResult irGenRegion(IRGS& irgs, } } + RegionDescPtr calleeRegion{nullptr}; + // See if we have a callee region we can inline---but only if the + // singleton inliner isn't actively inlining. + if (!skipTrans) { + calleeRegion = getInlinableCalleeRegion(psk, inst.funcd, retry, inl, + irgs); + } + + if (calleeRegion) { + always_assert(inst.op() == Op::FCall || inst.op() == Op::FCallD); + auto const* callee = inst.funcd; + + // We shouldn't be inlining profiling translations. + assert(mcg->tx().mode() != TransKind::Profile); + + FTRACE(1, "\nstarting inlined call from {} to {} with {} args " + "and stack:\n{}\n", + block->func()->fullName()->data(), + callee->fullName()->data(), + inst.imm[0].u_IVA, + show(irgs)); + + auto returnSk = inst.nextSk(); + auto returnFuncOff = returnSk.offset() - block->func()->base(); + + if (irgen::beginInlining(irgs, inst.imm[0].u_IVA, callee, + returnFuncOff)) { + SCOPE_ASSERT_DETAIL("Inlined-RegionDesc") + { return show(*calleeRegion); }; + + // Reset block state before reentering irGenRegion + irb.resetOffsetMapping(); + irb.resetGuardFailBlock(); + + auto result = irGenRegion(irgs, *calleeRegion, retry, trflags, inl); + inl.registerEndInlining(callee); + + if (result != TranslateResult::Success) { + // Generating the inlined call failed, bailout + return result; + } + + // If this block isn't empty create a new block for the remaining + // instructions + if (!lastInstr) { + auto nextBlock = irb.unit().defBlock(); + irb.setBlock(inst.nextSk(), nextBlock); + irgen::endBlock(irgs, inst.nextSk().offset(), inst.nextIsMerge); + + // Start a new IR block to hold the remainder of this block. + auto const did_start = + irb.startBlock(nextBlock, false /* unprocessedPred */); + always_assert_flog(did_start, + "Failed to start block following inlined region."); + } + + // Recursive calls to irGenRegion will reset the successor block + // mapping + setSuccIRBlocks(irgs, region, blockId, blockIdToIRBlock); + + // Don't emit the FCall + skipTrans = true; + } + } + // Emit IR for the body of the instruction. try { if (!skipTrans) { @@ -719,12 +799,12 @@ TranslateResult irGenRegion(IRGS& irgs, } } catch (const FailedIRGen& exn) { ProfSrcKey psk{irgs.profTransID, sk}; - always_assert_flog(!toInterp.count(psk), + always_assert_flog(!retry.toInterp.count(psk), "IR generation failed with {}\n", exn.what()); FTRACE(1, "ir generation for {} failed with {}\n", inst.toString(), exn.what()); - toInterp.insert(psk); + retry.toInterp.insert(psk); return TranslateResult::Retry; } @@ -740,17 +820,16 @@ TranslateResult irGenRegion(IRGS& irgs, // to be translated. if (lastInstr) { if (region.isExit(blockId)) { - irgen::endRegion(irgs); + if (!inl.inlining()) { + irgen::endRegion(irgs); + } else { + assertx(isReturnish(inst.op())); + } } else if (instrAllowsFallThru(inst.op())) { if (region.isSideExitingBlock(blockId)) { irgen::prepareForSideExit(irgs); } irgen::endBlock(irgs, inst.nextSk().offset(), inst.nextIsMerge); - } else if (isRet(inst.op()) || inst.op() == OpNativeImpl) { - // "Fallthrough" from inlined return to the next block - auto const callerBlock = region.block(singleSucc(region, blockId)); - irgen::endBlock(irgs, callerBlock->start().offset(), - inst.nextIsMerge); } } } @@ -760,14 +839,21 @@ TranslateResult irGenRegion(IRGS& irgs, assertx(!byRefs.hasNext()); assertx(!knownFuncs.hasNext()); } - irgen::sealUnit(irgs); + + if (!inl.inlining()) { + irgen::sealUnit(irgs); + } else { + always_assert_flog(irgs.inlineLevel == inl.depth() - 1, + "Tried to inline a region with no return."); + } + irGenTimer.stop(); return TranslateResult::Success; } TranslateResult mcGenRegion(IRGS& irgs, const RegionDesc& region, - RegionBlacklist& toInterp) { + ProfSrcKeySet& toInterp) { auto const startSk = region.start(); try { mcg->traceCodeGen(irgs); @@ -808,13 +894,17 @@ TranslateResult mcGenRegion(IRGS& irgs, TranslateResult translateRegion(IRGS& irgs, const RegionDesc& region, - RegionBlacklist& toInterp, + TranslateRetryContext& retry, TransFlags trflags, PostConditions& pConds) { SCOPE_ASSERT_DETAIL("RegionDesc") { return show(region); }; SCOPE_ASSERT_DETAIL("IRUnit") { return show(irgs.unit); }; - auto irGenResult = irGenRegion(irgs, region, toInterp, trflags); + // Set up inlining context, but disable it for profiling mode. + InliningDecider inl(region.entry()->func()); + if (mcg->tx().mode() == TransKind::Profile) inl.disable(); + + auto irGenResult = irGenRegion(irgs, region, retry, trflags, inl); if (irGenResult != TranslateResult::Success) return irGenResult; // For profiling translations, grab the postconditions to be used @@ -832,7 +922,7 @@ TranslateResult translateRegion(IRGS& irgs, pConds = irgs.irb->postConds(mainExit); } - return mcGenRegion(irgs, region, toInterp); + return mcGenRegion(irgs, region, retry.toInterp); } } } diff --git a/hphp/runtime/vm/jit/translate-region.h b/hphp/runtime/vm/jit/translate-region.h index 412993f1846..de130b6df77 100644 --- a/hphp/runtime/vm/jit/translate-region.h +++ b/hphp/runtime/vm/jit/translate-region.h @@ -34,23 +34,29 @@ enum class TranslateResult { const char* show(TranslateResult); /* - * Blacklisted instruction set. - * - * Used by translateRegion() to track instructions that must be interpreted. + * Data used by translateRegion() to pass information between retries. */ -using RegionBlacklist = ProfSrcKeySet; +struct TranslateRetryContext { + // Instructions that must be interpreted. + ProfSrcKeySet toInterp; + + // Inlined regions. + std::unordered_map inlines; +}; /* * Translate `region'. * - * The `toInterp' RegionBlacklist is a set of instructions which must be - * interpreted. When an instruction fails in translation, Retry is returned, - * and the instruction is added to `interp' so that it will be interpreted on - * the next attempt. + * The caller is expected to continue calling translateRegion() until either + * Success or Failure is returned. Otherwise, Retry is returned, and the + * caller is responsible for threading the same RetryContext through to the + * retried translations. */ -TranslateResult translateRegion(IRGS&, - const RegionDesc&, - RegionBlacklist& toInterp, +TranslateResult translateRegion(IRGS& irgs, + const RegionDesc& region, + TranslateRetryContext& retry, TransFlags trflags, PostConditions& pconds); diff --git a/hphp/test/quick/exceptions_clone.php.ini b/hphp/test/quick/exceptions_clone.php.ini new file mode 100644 index 00000000000..b399303bfc5 --- /dev/null +++ b/hphp/test/quick/exceptions_clone.php.ini @@ -0,0 +1 @@ +hhvm.hhir_enable_gen_time_inlining=false -- 2.11.4.GIT