From 3d9786b8cfba6cf60b830e367b11f009f93ec59e Mon Sep 17 00:00:00 2001 From: Brett Simmers Date: Fri, 12 Dec 2014 21:01:11 -0800 Subject: [PATCH] Support rVmFp modifications in llvm backend Summary: I considered giving it the same treatment I gave rVmSp in D1622952 but that would've required making changes to hundreds of hhir opcodes to add an explicit FramePtr src. This ended up being fairly simple and should work just as well. Reviewed By: @maksfb Differential Revision: D1735985 --- hphp/runtime/vm/jit/code-gen-x64.cpp | 22 ++++-------- hphp/runtime/vm/jit/service-requests-x64.cpp | 16 ++++----- hphp/runtime/vm/jit/service-requests-x64.h | 3 +- hphp/runtime/vm/jit/vasm-arm.cpp | 3 -- hphp/runtime/vm/jit/vasm-llvm.cpp | 51 ++++++++++++++++++---------- hphp/runtime/vm/jit/vasm-reg.h | 1 + hphp/runtime/vm/jit/vasm-x64.cpp | 4 --- hphp/runtime/vm/jit/vasm-x64.h | 9 +++-- 8 files changed, 57 insertions(+), 52 deletions(-) diff --git a/hphp/runtime/vm/jit/code-gen-x64.cpp b/hphp/runtime/vm/jit/code-gen-x64.cpp index 2ea37f1a389..682bec223b6 100644 --- a/hphp/runtime/vm/jit/code-gen-x64.cpp +++ b/hphp/runtime/vm/jit/code-gen-x64.cpp @@ -2230,7 +2230,6 @@ void CodeGenerator::cgRetCtrl(IRInstruction* inst) { auto sp = srcLoc(inst, 0).reg(); auto fp = srcLoc(inst, 1).reg(); v << syncvmsp{sp}; - v << syncvmfp{fp}; // Return control to caller if (RuntimeOption::EvalHHIRGenerateAsserts) { @@ -2454,7 +2453,6 @@ void CodeGenerator::cgStLocNT(IRInstruction* inst) { void CodeGenerator::cgSyncABIRegs(IRInstruction* inst) { auto& v = vmain(); - v << syncvmfp{srcLoc(inst, 0).reg()}; v << syncvmsp{srcLoc(inst, 1).reg()}; } @@ -3203,7 +3201,6 @@ void CodeGenerator::cgCallArray(IRInstruction* inst) { auto pc = v.cns(inst->extra()->pc); auto after = v.cns(inst->extra()->after); auto target = mcg->tx().uniqueStubs.fcallArrayHelper; - v << syncvmfp{srcLoc(inst, 1 /* fp */).reg()}; v << syncvmsp{srcLoc(inst, 0 /* sp */).reg()}; v << copy2{pc, after, argNumToRegName[0], argNumToRegName[1]}; v << callstub{target, argSet(2) | kCrossTraceRegs, @@ -3212,8 +3209,9 @@ void CodeGenerator::cgCallArray(IRInstruction* inst) { } void CodeGenerator::cgCall(IRInstruction* inst) { - auto const rSP = srcLoc(inst, 0).reg(); - auto const rFP = srcLoc(inst, 1).reg(); + auto const rSP = srcLoc(inst, 0).reg(); + auto const rFP = srcLoc(inst, 1).reg(); + auto const rNewSP = dstLoc(inst, 0).reg(); auto const srcKey = inst->marker().sk(); auto const extra = inst->extra(); auto const callee = extra->callee; @@ -3223,14 +3221,13 @@ void CodeGenerator::cgCall(IRInstruction* inst) { auto const ar = argc * sizeof(TypedValue); v << store{rFP, rSP[ar + AROFF(m_sfp)]}; v << storeli{safe_cast(extra->after), rSP[ar + AROFF(m_soff)]}; - v << syncvmfp{rFP}; - v << syncvmsp{rSP}; if (isNativeImplCall(callee, argc)) { - emitCallNativeImpl(v, vcold(), srcKey, callee, argc); + emitCallNativeImpl(v, vcold(), srcKey, callee, argc, rSP, rNewSP); } else { + v << syncvmsp{rSP}; emitBindCall(v, m_state.frozen, callee, argc); + v << defvmsp{rNewSP}; } - v << defvmsp{dstLoc(inst, 0).reg()}; } void CodeGenerator::cgCastStk(IRInstruction *inst) { @@ -4012,7 +4009,6 @@ void CodeGenerator::cgGuardStk(IRInstruction* inst) { auto const rSP = srcLoc(inst, 0).reg(); auto const baseOff = cellsToBytes(inst->extra()->offset); auto& v = vmain(); - v << syncvmfp{srcLoc(inst, 1).reg()}; v << syncvmsp{srcLoc(inst, 0).reg()}; emitTypeGuard(inst->marker(), inst->typeParam(), rSP[baseOff + TVOFF(m_type)], @@ -4033,7 +4029,6 @@ void CodeGenerator::cgGuardLoc(IRInstruction* inst) { auto const rFP = srcLoc(inst, 0).reg(); auto const baseOff = localOffset(inst->extra()->locId); auto& v = vmain(); - v << syncvmfp{srcLoc(inst, 0).reg()}; v << syncvmsp{srcLoc(inst, 1).reg()}; emitTypeGuard(inst->marker(), inst->typeParam(), rFP[baseOff + TVOFF(m_type)], @@ -4294,12 +4289,11 @@ void CodeGenerator::emitReffinessTest(IRInstruction* inst, Vreg sf, void CodeGenerator::cgGuardRefs(IRInstruction* inst) { auto& v = vmain(); auto const sf = v.makeReg(); - v << syncvmfp{srcLoc(inst, 5 /* fp */).reg()}; - v << syncvmsp{srcLoc(inst, 6 /* sp */).reg()}; emitReffinessTest(inst, sf, [&](Vout& v, ConditionCode cc, Vreg sfTaken) { auto& marker = inst->marker(); auto dest = SrcKey(getFunc(marker), marker.bcOff(), resumed(marker)); + v << syncvmsp{srcLoc(inst, 6 /* sp */).reg()}; v << fallbackcc{cc, sfTaken, dest, TransFlags(), kCrossTraceRegs}; }); } @@ -4964,7 +4958,6 @@ void CodeGenerator::cgInterpOneCF(IRInstruction* inst) { auto sp = dstLoc(inst, 0).reg(); v << load{rVmTl[RDS::kVmfpOff], fp}; v << load{rVmTl[RDS::kVmspOff], sp}; - v << syncvmfp{fp}; v << syncvmsp{sp}; emitServiceReq(v, nullptr, REQ_RESUME, {}); } @@ -4980,7 +4973,6 @@ void CodeGenerator::cgContEnter(IRInstruction* inst) { v << store{curFpReg, genFpReg[AROFF(m_sfp)]}; v << storeli{returnOff, genFpReg[AROFF(m_soff)]}; v << copy{genFpReg, curFpReg}; - v << syncvmfp{curFpReg}; v << syncvmsp{curSpReg}; v << contenter{curFpReg, addrReg, kCrossTraceRegs}; // curFpReg->m_savedRip will point here, and the next HHIR opcode must diff --git a/hphp/runtime/vm/jit/service-requests-x64.cpp b/hphp/runtime/vm/jit/service-requests-x64.cpp index b0ceff4c717..c4b3268f59d 100644 --- a/hphp/runtime/vm/jit/service-requests-x64.cpp +++ b/hphp/runtime/vm/jit/service-requests-x64.cpp @@ -229,16 +229,14 @@ TCA emitRetranslate(CodeBlock& cb, CodeBlock& frozen, jit::ConditionCode cc, } void emitCallNativeImpl(Vout& v, Vout& vc, SrcKey srcKey, const Func* func, - int numArgs) { + int numArgs, Vreg inSp, Vreg outSp) { assert(isNativeImplCall(func, numArgs)); auto retAddr = (int64_t)mcg->tx().uniqueStubs.retHelper; - v << store{v.cns(retAddr), rVmSp[cellsToBytes(numArgs) + AROFF(m_savedRip)]}; + v << store{v.cns(retAddr), inSp[cellsToBytes(numArgs) + AROFF(m_savedRip)]}; assert(numArgs == func->numLocals()); assert(func->numIterators() == 0); - v << lea{rVmSp[cellsToBytes(numArgs)], rVmFp}; + v << lea{inSp[cellsToBytes(numArgs)], rVmFp}; emitCheckSurpriseFlagsEnter(v, vc, Fixup{0, numArgs}); - // rVmSp is already correctly adjusted, because there's no locals - // other than the arguments passed. BuiltinFunction builtinFuncPtr = func->builtinFuncPtr(); if (false) { // typecheck ActRec* ar = nullptr; @@ -254,7 +252,7 @@ void emitCallNativeImpl(Vout& v, Vout& vc, SrcKey srcKey, const Func* func, */ if (mcg->fixupMap().eagerRecord(func)) { emitEagerSyncPoint(v, reinterpret_cast(func->getEntry()), - rVmFp, rVmSp); + rVmFp, inSp); } v << vcall{CppCall::direct(builtinFuncPtr), v.makeVcallArgs({{rVmFp}}), v.makeTuple({}), Fixup{0, numArgs}}; @@ -289,9 +287,7 @@ void emitCallNativeImpl(Vout& v, Vout& vc, SrcKey srcKey, const Func* func, emitRB(v, Trace::RBTypeFuncExit, func->fullName()->data()); auto adjust = safe_cast(sizeof(ActRec) + cellsToBytes(nLocalCells-1)); - if (adjust) { - v << addqi{adjust, rVmSp, rVmSp, v.makeReg()}; - } + v << lea{inSp[adjust], outSp}; } /* @@ -312,7 +308,7 @@ void emitBindCall(Vout& v, CodeBlock& frozen, emitImmStoreq(v, kUninitializedRIP, rVmSp[off]); } v << lea{rVmSp[cellsToBytes(numArgs)], rStashedAR}; - v << bindcall{addr, RegSet(rStashedAR)}; + v << bindcall{addr, kCrossCallRegs}; } }}} diff --git a/hphp/runtime/vm/jit/service-requests-x64.h b/hphp/runtime/vm/jit/service-requests-x64.h index d3b1dea9504..70a1ea37042 100644 --- a/hphp/runtime/vm/jit/service-requests-x64.h +++ b/hphp/runtime/vm/jit/service-requests-x64.h @@ -17,6 +17,7 @@ #define incl_HPHP_JIT_SERVICE_REQUESTS_X64_H_ #include "hphp/runtime/vm/jit/service-requests.h" +#include "hphp/runtime/vm/jit/vasm-reg.h" #include "hphp/util/asm-x64.h" #include "hphp/util/data-block.h" @@ -64,7 +65,7 @@ TCA emitRetranslate(CodeBlock& cb, CodeBlock& frozen, jit::ConditionCode cc, void emitBindCall(Vout& v, CodeBlock& frozen, const Func* funcd, int numArgs); void emitCallNativeImpl(Vout& v, Vout& vc, SrcKey srcKey, const Func* funcd, - int numArgs); + int numArgs, Vreg inSp, Vreg outSp); // An intentionally funny-looking-in-core-dumps constant for uninitialized // instruction pointers. diff --git a/hphp/runtime/vm/jit/vasm-arm.cpp b/hphp/runtime/vm/jit/vasm-arm.cpp index fb76f3c6781..3e3aadc9592 100644 --- a/hphp/runtime/vm/jit/vasm-arm.cpp +++ b/hphp/runtime/vm/jit/vasm-arm.cpp @@ -571,9 +571,6 @@ void lower(Vunit& unit) { case Vinstr::syncvmsp: inst = copy{inst.syncvmsp_.s, PhysReg{arm::rVmSp}}; break; - case Vinstr::syncvmfp: - inst = copy{inst.syncvmfp_.s, PhysReg{arm::rVmFp}}; - break; default: break; } diff --git a/hphp/runtime/vm/jit/vasm-llvm.cpp b/hphp/runtime/vm/jit/vasm-llvm.cpp index 0caacaffc9d..01cc8097fbd 100644 --- a/hphp/runtime/vm/jit/vasm-llvm.cpp +++ b/hphp/runtime/vm/jit/vasm-llvm.cpp @@ -447,6 +447,7 @@ struct LLVMEmitter { , m_tcMM(new TCMemoryManager(areas)) , m_valueInfo(unit.next_vr) , m_blocks(unit.blocks.size()) + , m_incomingVmFp(unit.blocks.size()) , m_unit(unit) , m_areas(areas) { @@ -474,11 +475,12 @@ struct LLVMEmitter { } auto args = m_function->arg_begin(); - m_valueInfo[Vreg(x64::rVmSp)].llval = args++; - m_rVmTl = m_valueInfo[Vreg(x64::rVmTl)].llval = args++; - m_rVmTl->setName("rVmTl"); - m_rVmFp = m_valueInfo[Vreg(x64::rVmFp)].llval = args++; - m_rVmFp->setName("rVmFp"); + args->setName("rVmSp"); + defineValue(x64::rVmSp, args++); + args->setName("rVmTl"); + defineValue(x64::rVmTl, args++); + args->setName("rVmFp"); + defineValue(x64::rVmFp, args++); // Commonly used types and values. m_int8 = m_irb.getInt8Ty(); @@ -748,7 +750,7 @@ struct LLVMEmitter { */ llvm::Value* value(Vreg tmp) const { auto& info = m_valueInfo.at(tmp); - always_assert(info.llval); + always_assert_flog(info.llval, "No llvm value exists for {}", show(tmp)); return info.llval; } @@ -1028,6 +1030,9 @@ VASM_OPCODES // Vlabel -> llvm::BasicBlock map jit::vector m_blocks; + // Vlabel -> Value for rVmFp entering a vasm block + jit::vector m_incomingVmFp; + jit::hash_map m_phiInfos; // Pending Fixups that must be processed after codegen @@ -1047,10 +1052,6 @@ VASM_OPCODES const Vunit& m_unit; Vasm::AreaList& m_areas; - // Special regs. - llvm::Value* m_rVmTl{nullptr}; - llvm::Value* m_rVmFp{nullptr}; - // Faux personality for emitting landingpad. llvm::Function* m_personalityFunc; @@ -1099,8 +1100,13 @@ void LLVMEmitter::emit(const jit::vector& labels) { auto& b = m_unit.blocks[label]; m_irb.SetInsertPoint(block(label)); - // TODO(#5376594): before rVmFp is SSA-ified we are using the hack below. - m_valueInfo[Vreg(x64::rVmFp)].llval = m_rVmFp; + // If this isn't the first block of the unit, load the incoming value of + // rVmFp from our pred(s). + if (label != labels.front()) { + auto& inFp = m_incomingVmFp[label]; + always_assert(inFp); + defineValue(x64::rVmFp, inFp); + } for (auto& inst : b.code) { if (traceInstrs) { @@ -1220,7 +1226,6 @@ O(subq) \ O(subqi) \ O(subsd) \ O(svcreq) \ -O(syncvmfp) \ O(syncvmsp) \ O(testb) \ O(testbi) \ @@ -1306,6 +1311,22 @@ O(phidef) defineValue(def, inst); }); } + + // Since rVmFp isn't SSA in vasm code, we manually propagate its value + // across control flow edges. + for (auto label : succs(b)) { + auto& inFp = m_incomingVmFp[label]; + // If this assertion fails it's either a bug in the incoming code or it + // just means we'll need to insert phi nodes to merge the value. + always_assert(inFp == nullptr || inFp == value(x64::rVmFp)); + inFp = value(x64::rVmFp); + } + + // Any values for these ABI registers aren't meant to last past the end of + // the block. + defineValue(x64::rVmSp, nullptr); + defineValue(x64::rVmFp, nullptr); + defineValue(x64::rStashedAR, nullptr); } finalize(); @@ -2389,10 +2410,6 @@ void LLVMEmitter::emit(const svcreq& inst) { m_irb.CreateRetVoid(); } -void LLVMEmitter::emit(const syncvmfp& inst) { - // Nothing to do really. -} - void LLVMEmitter::emit(const syncvmsp& inst) { defineValue(x64::rVmSp, value(inst.s)); } diff --git a/hphp/runtime/vm/jit/vasm-reg.h b/hphp/runtime/vm/jit/vasm-reg.h index 1fa615c3e53..8f5c6dd5492 100644 --- a/hphp/runtime/vm/jit/vasm-reg.h +++ b/hphp/runtime/vm/jit/vasm-reg.h @@ -17,6 +17,7 @@ #ifndef incl_HPHP_JIT_VASM_REG_H_ #define incl_HPHP_JIT_VASM_REG_H_ +#include "hphp/runtime/vm/jit/phys-reg.h" #include "hphp/runtime/vm/jit/vasm.h" namespace HPHP { namespace jit { diff --git a/hphp/runtime/vm/jit/vasm-x64.cpp b/hphp/runtime/vm/jit/vasm-x64.cpp index fb567b6b97a..5264d6dc263 100644 --- a/hphp/runtime/vm/jit/vasm-x64.cpp +++ b/hphp/runtime/vm/jit/vasm-x64.cpp @@ -1144,10 +1144,6 @@ static void lowerForX64(Vunit& unit, const Abi& abi) { inst = copy{inst.syncvmsp_.s, rVmSp}; break; - case Vinstr::syncvmfp: - inst = copy{inst.syncvmfp_.s, rVmFp}; - break; - case Vinstr::ldretaddr: inst = pushm{inst.ldretaddr_.s}; break; diff --git a/hphp/runtime/vm/jit/vasm-x64.h b/hphp/runtime/vm/jit/vasm-x64.h index 14f9e59a264..d4edefee8b1 100644 --- a/hphp/runtime/vm/jit/vasm-x64.h +++ b/hphp/runtime/vm/jit/vasm-x64.h @@ -330,7 +330,6 @@ inline Vptr Vr::operator+(size_t d) const { O(landingpad, Inone, Un, Dn)\ O(defvmsp, Inone, Un, D(d))\ O(syncvmsp, Inone, U(s), Dn)\ - O(syncvmfp, Inone, U(s), Dn)\ O(srem, Inone, U(s0) U(s1), D(d))\ O(sar, Inone, U(s0) U(s1), D(d) D(sf))\ O(shl, Inone, U(s0) U(s1), D(d) D(sf))\ @@ -520,9 +519,15 @@ struct svcreq { ServiceRequest req; Vtuple args; TCA stub_block; }; struct syncpoint { Fixup fix; }; struct unwind { Vlabel targets[2]; }; struct landingpad {}; + +/* Copy rVmSp into d. Used when reentering translated code after an ABI + * boundary, such as the beginning of a tracelet or right after a bindcall. */ struct defvmsp { Vreg d; }; + +/* Copy s into rVmSp. Used right before leaving translated code for an ABI + * boundary, such as bindjmp or fallbackcc. */ struct syncvmsp { Vreg s; }; -struct syncvmfp { Vreg s; }; + struct srem { Vreg s0, s1, d; }; struct sar { Vreg s0, s1, d; VregSF sf; }; struct shl { Vreg s0, s1, d; VregSF sf; }; -- 2.11.4.GIT