From 957b19d088311d52aa06d5c07564b3845558b3bf Mon Sep 17 00:00:00 2001 From: Edwin Smith Date: Sun, 5 May 2013 04:29:14 -0700 Subject: [PATCH] Move m_liveRegs out of IRInstruction The live regs field in IRInstruction is the result of an analysis pass which can be better encapsulated by moving the field into its own StateVector. --- hphp/runtime/vm/translator/hopt/codegen.cpp | 65 +++++++++++++++---- hphp/runtime/vm/translator/hopt/codegen.h | 20 +++++- hphp/runtime/vm/translator/hopt/ir.cpp | 2 - hphp/runtime/vm/translator/hopt/ir.h | 8 +-- .../runtime/vm/translator/hopt/linearscan.cpp | 30 --------- 5 files changed, 73 insertions(+), 52 deletions(-) diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 741f48390..7c1e05ac9 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -810,13 +810,25 @@ void CodeGenerator::cgCallHelper(Asm& a, SyncOptions sync, ArgGroup& args, DestType destType) { + cgCallHelper(a, call, dstReg0, dstReg1, sync, args, + m_state.liveRegs[m_curInst], destType); +} + +void CodeGenerator::cgCallHelper(Asm& a, + const Transl::Call& call, + PhysReg dstReg0, + PhysReg dstReg1, + SyncOptions sync, + ArgGroup& args, + RegSet toSave, + DestType destType) { assert(int(args.size()) <= kNumRegisterArgs); assert(m_curInst->isNative()); // Save the register that are live at the point after this IR instruction. // However, don't save the destination registers that will be overwritten // by this call. - RegSet toSave = m_curInst->getLiveRegs() & kCallerSaved; + toSave = toSave & kCallerSaved; assert((toSave & RegSet().add(dstReg0).add(dstReg1)).empty()); PhysRegSaverParity<1> regSaver(a, toSave); @@ -2684,7 +2696,7 @@ void CodeGenerator::cgGenericRetDecRefs(IRInstruction* inst) { * are still allocated to registers at this time. */ const auto UNUSED expectedLiveRegs = RegSet(rFp).add(rDest) | retvalRegs; - assert((m_curInst->getLiveRegs() - expectedLiveRegs).empty()); + assert((m_state.liveRegs[m_curInst] - expectedLiveRegs).empty()); assert(rFp == rVmFp && "free locals helper assumes the frame pointer is rVmFp"); assert(rDest == rVmSp && @@ -2999,7 +3011,7 @@ void CodeGenerator::cgDecRefDynamicTypeMem(PhysReg baseReg, if (exit == nullptr && RuntimeOption::EvalHHIRGenericDtorHelper) { { // This PhysRegSaverParity saves rdi redundantly if - // !m_curInst->getLiveRegs().contains(rdi), but its + // !liveRegs[m_curInst].contains(rdi), but its // necessary to maintain stack alignment. We can do better // by making the helpers adjust the stack for us in the cold // path, which calls the destructor. @@ -4256,18 +4268,17 @@ void CodeGenerator::cgLdClsMethodFCache(IRInstruction* inst) { // Handle case where method is not entered in the cache unlikelyIfBlock(CC_E, [&] (Asm& a) { - if (false) { // typecheck - const UNUSED Func* f = StaticMethodFCache::lookupIR(ch, cls, methName); - } + const Func* (*lookup)(CacheHandle, const Class*, const StringData*) = + StaticMethodFCache::lookupIR; // preserve destCtxReg across the call since it wouldn't be otherwise - inst->setLiveRegs(inst->getLiveRegs().add(destCtxReg)); - cgCallHelper(a, - (TCA)StaticMethodFCache::lookupIR, - funcDestReg, + RegSet toSave = m_state.liveRegs[inst] | RegSet(destCtxReg); + cgCallHelper(a, Transl::Call((TCA)lookup), + funcDestReg, InvalidReg, kSyncPoint, ArgGroup().imm(ch) .immPtr(cls) - .immPtr(methName)); + .immPtr(methName), + toSave); // If entry found in target cache, jump back to m_as. // Otherwise, bail to exit label a.testq(funcDestReg, funcDestReg); @@ -5146,6 +5157,35 @@ void CodeGenerator::print() const { JIT::print(std::cout, m_curTrace, m_state.asmInfo); } +/* + * Compute and save registers that are live *across* each inst, not including + * registers whose lifetimes end at inst, nor registers defined by inst. + */ +LiveRegs computeLiveRegs(const IRFactory* factory, Block* start_block) { + StateVector liveMap(factory, RegSet()); + LiveRegs live_regs(factory, RegSet()); + postorderWalk( + [&](Block* block) { + RegSet& live = liveMap[block]; + if (Block* taken = block->getTaken()) live = liveMap[taken]; + if (Block* next = block->getNext()) live |= liveMap[next]; + for (auto it = block->end(); it != block->begin(); ) { + IRInstruction& inst = *--it; + for (const SSATmp& dst : inst.getDsts()) { + live -= dst.getRegs(); + } + live_regs[inst] = live; + for (SSATmp* src : inst.getSrcs()) { + live |= src->getRegs(); + } + } + }, + factory->numBlocks(), + start_block + ); + return live_regs; +} + // select instructions for the trace and its exits void genCodeForTrace(Trace* trace, CodeGenerator::Asm& as, @@ -5155,7 +5195,8 @@ void genCodeForTrace(Trace* trace, Transl::TranslatorX64* tx64, AsmInfo* asmInfo) { assert(trace->isMain()); - CodegenState state(irFactory, asmInfo); + LiveRegs live_regs = computeLiveRegs(irFactory, trace->front()); + CodegenState state(irFactory, live_regs, asmInfo); cgTrace(trace, as, astubs, tx64, bcMap, state); for (Trace* exit : trace->getExitTraces()) { cgTrace(exit, astubs, astubs, tx64, nullptr, state); diff --git a/hphp/runtime/vm/translator/hopt/codegen.h b/hphp/runtime/vm/translator/hopt/codegen.h index bade0ea5d..9287dbc7d 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.h +++ b/hphp/runtime/vm/translator/hopt/codegen.h @@ -67,12 +67,16 @@ struct AsmInfo { StateVector astubRanges; }; +typedef StateVector LiveRegs; + // Stuff we need to preserve between blocks while generating code, // and address information produced during codegen. struct CodegenState { - CodegenState(const IRFactory* factory, AsmInfo* asmInfo) + CodegenState(const IRFactory* factory, const LiveRegs& liveRegs, + AsmInfo* asmInfo) : patches(factory, nullptr) , lastMarker(nullptr) + , liveRegs(liveRegs) , asmInfo(asmInfo) {} @@ -87,6 +91,12 @@ struct CodegenState { // next block in the same assmbler. bool noTerminalJmp_; + // for each instruction, holds the RegSet of registers that must be + // preserved across that instruction. This is for push/pop of caller-saved + // registers. + const LiveRegs& liveRegs; + + // Output: start/end ranges of machine code addresses of each instruction. AsmInfo* asmInfo; }; @@ -143,6 +153,14 @@ private: SyncOptions sync, ArgGroup& args, DestType destType = DestType::SSA); + void cgCallHelper(Asm& a, + const Transl::Call& call, + PhysReg dstReg0, + PhysReg dstReg1, + SyncOptions sync, + ArgGroup& args, + RegSet toSave, + DestType destType = DestType::SSA); void cgStore(PhysReg base, int64_t off, diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index 43a4a0ce8..41d1f48ae 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -599,7 +599,6 @@ void IRInstruction::convertToNop() { m_numSrcs = nop.m_numSrcs; m_id = nop.m_id; m_srcs = nop.m_srcs; - m_liveRegs = nop.m_liveRegs; m_numDsts = nop.m_numDsts; m_dst = nop.m_dst; m_taken = nullptr; @@ -637,7 +636,6 @@ void IRInstruction::become(IRFactory* factory, IRInstruction* other) { m_op = other->m_op; m_typeParam = other->m_typeParam; m_id = other->m_id; - m_liveRegs = other->m_liveRegs; m_taken = other->m_taken; m_tca = other->m_tca; m_numSrcs = other->m_numSrcs; diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index e4b6ca128..7cd707fcd 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -1852,11 +1852,6 @@ struct IRInstruction { */ bool isTransient() const { return m_iid == kTransient; } - // LiveRegs is the set of registers that are live across this instruction. - // Doesn't include dest registers, or src registers whose lifetime ends here. - RegSet getLiveRegs() const { return m_liveRegs; } - void setLiveRegs(RegSet s) { m_liveRegs = s; } - Block* getBlock() const { return m_block; } void setBlock(Block* b) { m_block = b; } Trace* getTrace() const; @@ -1919,7 +1914,6 @@ private: const IId m_iid; uint32_t m_id; SSATmp** m_srcs; - RegSet m_liveRegs; SSATmp* m_dst; // if HasDest or NaryDest Block* m_taken; // for branches, guards, and jmp Block* m_block; // block that owns this instruction @@ -2319,7 +2313,7 @@ struct Block : boost::noncopyable { // list-compatible interface; these delegate to m_instrs but also update // inst.m_block InstructionList& getInstrs() { return m_instrs; } - bool empty() { return m_instrs.empty(); } + bool empty() const { return m_instrs.empty(); } iterator begin() { return m_instrs.begin(); } iterator end() { return m_instrs.end(); } const_iterator begin() const { return m_instrs.begin(); } diff --git a/hphp/runtime/vm/translator/hopt/linearscan.cpp b/hphp/runtime/vm/translator/hopt/linearscan.cpp index b7aab156f..04ad4ec21 100644 --- a/hphp/runtime/vm/translator/hopt/linearscan.cpp +++ b/hphp/runtime/vm/translator/hopt/linearscan.cpp @@ -113,7 +113,6 @@ private: void allocRegToTmp(SSATmp* ssaTmp, uint32_t index); void freeRegsAtId(uint32_t id); void spill(SSATmp* tmp); - void computeLiveRegs(); template SSATmp* cns(T val) { return m_irFactory->cns(val); @@ -256,32 +255,6 @@ LinearScan::LinearScan(IRFactory* irFactory) } } -/* - * Compute and save registers that are live *across* each inst, not including - * registers whose lifetimes end at inst, nor registers defined by inst. - */ -void LinearScan::computeLiveRegs() { - StateVector liveMap(m_irFactory, RegSet()); - postorderWalk( - [&](Block* block) { - RegSet& live = liveMap[block]; - if (Block* taken = block->getTaken()) live = liveMap[taken]; - if (Block* next = block->getNext()) live |= liveMap[next]; - for (auto it = block->end(); it != block->begin(); ) { - IRInstruction& inst = *--it; - for (const SSATmp& dst : inst.getDsts()) { - RegSet d = dst.getRegs(); - live -= d; - } - inst.setLiveRegs(live); - for (SSATmp* src : inst.getSrcs()) { - live |= src->getRegs(); - } - } - }, - m_irFactory->numBlocks(), m_blocks.front()); -} - template static inline void dumpIR(const Inner* in, const char* msg) { if (dumpIREnabled(DumpVal)) { @@ -1033,9 +1006,6 @@ void LinearScan::allocRegs(Trace* trace) { } if (m_slots.size()) genSpillStats(trace, numSpillLocs); - - // record the live register set at each instruction - computeLiveRegs(); } void LinearScan::allocRegsOneTrace(BlockList::iterator& blockIt,