From ea03ae9b15cb2d302674b9395cd00535a89a6f99 Mon Sep 17 00:00:00 2001 From: alia Date: Mon, 18 Mar 2013 14:09:42 -0700 Subject: [PATCH] HHIR: Optimized useless incref-decref pairs. Incref-Decref pairs whose incref'ed SSATmp is not used by any other instruction other than the decref can be eliminated when the type of the refcounted value has no destructor (i.e., strings and boxed strings). This commonly occurs for stores to memory (e.g., SetS, SetG, and SetM) followed by PopC. For example: 47: SetS (32) t16:PtrToGen = LdClsPropAddrCached t9:Cls, "s1", "c", Cls(0) (33) t17:PtrToCell = UnboxPtr t16:PtrToGen (34) t18:Cell = LdMem t17:PtrToCell, 0 (35) t19:Str = IncRef t15:Str (36) StMem [t17:PtrToCell]:Str, t15:Str (37) DecRef t18:Cell 48: PopC (39) DecRef t19:Str In this example, eliminating the DecRef instruction (39) along with its IncRef (35) may cause the string referenced by t19 to be destroyed earlier in the DecRef instruction (37), but this is safe because destroying strings has no visible side effects. This diff eliminates these pairs. Note that its not safe to do this for types that have destructors with side effects. Applying this optimization to decref'ed types that have destructors could change the order of those side effects with respect to the side effects in instruction (37). A follow-on diff will change the DecRef (37) to bail the trace when it has to run a destructor to enable this for the general case. --- hphp/runtime/vm/translator/hopt/codegen.cpp | 5 +-- hphp/runtime/vm/translator/hopt/dce.cpp | 45 +++++++++++++++---- hphp/runtime/vm/translator/hopt/ir.h | 14 +++--- .../vm/translator/hopt/tracebuilder.cpp | 2 +- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 0b1c05640..8d0d2ad24 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -4105,11 +4105,8 @@ void CodeGenerator::cgLookupClsCns(IRInstruction* inst) { StringData* fullName = fullConstName(cls, cnsName); TargetCache::CacheHandle ch = TargetCache::allocClassConstant(fullName); - auto rTvPtr = rScratch; // Cell* ptr to slot in target cache - m_as.lea(rVmTl[ch], rTvPtr); - ArgGroup args; - args.reg(rTvPtr) + args.addr(rVmTl, ch) .immPtr(Unit::GetNamedEntity(cls->getValStr())) .immPtr(cls->getValStr()) .immPtr(cnsName->getValStr()); diff --git a/hphp/runtime/vm/translator/hopt/dce.cpp b/hphp/runtime/vm/translator/hopt/dce.cpp index da8e7f0d5..a190339c5 100644 --- a/hphp/runtime/vm/translator/hopt/dce.cpp +++ b/hphp/runtime/vm/translator/hopt/dce.cpp @@ -145,13 +145,15 @@ BlockList removeUnreachable(Trace* trace, IRFactory* factory) { } WorkList -initInstructions(Trace* trace, const BlockList& blocks, - DceState& state, IRFactory* factory) { +initInstructions(const BlockList& blocks, DceState& state) { TRACE(5, "DCE:vvvvvvvvvvvvvvvvvvvv\n"); // mark reachable, essential, instructions live and enqueue them WorkList wl; - for (const Block* block : blocks) { - for (const IRInstruction& inst : *block) { + for (Block* block : blocks) { + for (IRInstruction& inst : *block) { + for (SSATmp& dst : inst.getDsts()) { + dst.setUseCount(0); + } if (inst.isControlFlowInstruction()) { // mark the destination label so that the destination trace // is marked reachable @@ -178,7 +180,10 @@ initInstructions(Trace* trace, const BlockList& blocks, // 1) Change all unconsumed IncRefs to Mov. // 2) Mark a conditionally dead DecRefNZ as live if its corresponding IncRef // cannot be eliminated. +// 3) Eliminates IncRef-DecRef pairs who value is used only by the DecRef and +// whose type does not run a destructor with side effects. void optimizeRefCount(Trace* trace, DceState& state) { + WorkList decrefs; forEachInst(trace, [&](IRInstruction* inst) { if (inst->getOpcode() == IncRef && !state[inst].countConsumedAny()) { auto& s = state[inst]; @@ -192,15 +197,36 @@ void optimizeRefCount(Trace* trace, DceState& state) { s.setDead(); } if (inst->getOpcode() == DecRefNZ) { - IRInstruction* srcInst = inst->getSrc(0)->getInstruction(); + SSATmp* src = inst->getSrc(0); + IRInstruction* srcInst = src->getInstruction(); if (state[srcInst].countConsumedAny()) { state[inst].setLive(); + src->incUseCount(); + } + } + if (inst->getOpcode() == DecRef) { + SSATmp* src = inst->getSrc(0); + if (src->getUseCount() == 1 && !src->getType().canRunDtor()) { + IRInstruction* srcInst = src->getInstruction(); + if (srcInst->getOpcode() == IncRef) { + decrefs.push_back(inst); + } } } // Do copyProp at last. When processing DecRefNZs, we still need to look at // its source which should not be trampled over. Simplifier::copyProp(inst); }); + for (const IRInstruction* decref : decrefs) { + assert(decref->getOpcode() == DecRef); + SSATmp* src = decref->getSrc(0); + assert(src->getInstruction()->getOpcode() == IncRef); + assert(!src->getType().canRunDtor()); + if (src->getUseCount() == 1) { + state[decref].setDead(); + state[src->getInstruction()].setDead(); + } + } } /* @@ -275,7 +301,7 @@ void sinkIncRefs(Trace* trace, IRFactory* irFactory, DceState& state) { state[inst].setDead(); // Put all REFCOUNT_CONSUMED_OFF_TRACE IncRefs to the sinking list. toSink.push_back(inst); - } else { + } else if (!state[inst].isDead()) { assert(state[inst].countConsumed()); } } @@ -334,7 +360,7 @@ void eliminateDeadCode(Trace* trace, IRFactory* irFactory) { // work list; this will also mark reachable exit traces. All // other instructions marked dead. DceState state(irFactory, DceFlags()); - WorkList wl = initInstructions(trace, blocks, state, irFactory); + WorkList wl = initInstructions(blocks, state); // process the worklist while (!wl.empty()) { @@ -342,10 +368,11 @@ void eliminateDeadCode(Trace* trace, IRFactory* irFactory) { wl.pop_front(); for (uint32_t i = 0; i < inst->getNumSrcs(); i++) { SSATmp* src = inst->getSrc(i); - if (src->getInstruction()->getOpcode() == DefConst) { + IRInstruction* srcInst = src->getInstruction(); + if (srcInst->getOpcode() == DefConst) { continue; } - IRInstruction* srcInst = src->getInstruction(); + src->incUseCount(); if (state[srcInst].isDead()) { state[srcInst].setLive(); wl.push_back(srcInst); diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 6ab39c31f..36aafca69 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -1558,10 +1558,10 @@ private: private: Opcode m_op; Type m_typeParam; - uint16_t m_numSrcs; - uint16_t m_numDsts; + uint16_t m_numSrcs; + uint16_t m_numDsts; const IId m_iid; - uint32_t m_id; + uint32_t m_id; SSATmp** m_srcs; RegSet m_liveOutRegs; SSATmp* m_dst; // if HasDest or NaryDest @@ -1626,17 +1626,17 @@ inline std::ostream& operator<<(std::ostream& os, SpillInfo si) { class SSATmp { public: - uint32_t getId() const { return m_id; } + uint32_t getId() const { return m_id; } IRInstruction* getInstruction() const { return m_inst; } void setInstruction(IRInstruction* i) { m_inst = i; } Type getType() const { return m_type; } void setType(Type t) { m_type = t; } - uint32_t getLastUseId() const { return m_lastUseId; } + uint32_t getLastUseId() const { return m_lastUseId; } void setLastUseId(uint32_t newId) { m_lastUseId = newId; } - uint32_t getUseCount() const { return m_useCount; } + uint32_t getUseCount() const { return m_useCount; } void setUseCount(uint32_t count) { m_useCount = count; } void incUseCount() { m_useCount++; } - uint32_t decUseCount() { return --m_useCount; } + uint32_t decUseCount() { return --m_useCount; } bool isBoxed() const { return getType().isBoxed(); } bool isString() const { return isA(Type::Str); } bool isArray() const { return isA(Type::Arr); } diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index f6b7160df..8f0b8df2d 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -295,7 +295,7 @@ Trace* TraceBuilder::genExitTrace(uint32_t bcOff, marker.bcOff = bcOff; marker.stackOff = m_spOffset + numOpnds - stackDeficit; marker.func = m_curFunc->getValFunc(); - gen(Marker, &marker); + exitTrace->back()->push_back(m_irFactory.gen(Marker, &marker)); SSATmp* sp = m_spValue; if (numOpnds != 0 || stackDeficit != 0) {