diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 92a4ec0c7..8847a6181 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -2954,6 +2954,11 @@ void CodeGenerator::cgDecRefNZ(IRInstruction* inst) { cgDecRefWork(inst, false); } +void CodeGenerator::cgDecRefNZOrBranch(IRInstruction* inst) { + assert(inst->getTaken()); + cgDecRefWork(inst, true); +} + void CodeGenerator::emitSpillActRec(SSATmp* sp, int64_t spOffset, SSATmp* defAR) { diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index e40c3845f..2b2a03b3e 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -2137,10 +2137,17 @@ SSATmp* HhbcTranslator::unboxPtr(SSATmp* ptr) { } void HhbcTranslator::emitBindMem(SSATmp* ptr, SSATmp* src) { - SSATmp* prevValue = m_tb->genLdMem(ptr, Type::Gen, NULL); + SSATmp* prevValue = m_tb->genLdMem(ptr, ptr->getType().deref(), NULL); pushIncRef(src); m_tb->genStMem(ptr, src, true); - m_tb->genDecRef(prevValue); + if (isRefCounted(src) && src->getType().canRunDtor()) { + Block* exitBlock = getExitTrace(getNextSrcKey().offset())->front(); + Block::iterator markerInst = exitBlock->skipLabel(); + exitBlock->insert(++markerInst, m_irFactory.gen(DecRef, prevValue)); + m_tb->gen(DecRefNZOrBranch, exitBlock, prevValue); + } else { + m_tb->genDecRef(prevValue); + } } template @@ -2153,11 +2160,7 @@ void HhbcTranslator::emitBind(const StringData* name, } void HhbcTranslator::emitSetMem(SSATmp* ptr, SSATmp* src) { - ptr = unboxPtr(ptr); - SSATmp* prevValue = m_tb->genLdMem(ptr, Type::Cell, nullptr); - pushIncRef(src); - m_tb->genStMem(ptr, src, true); - m_tb->genDecRef(prevValue); + emitBindMem(unboxPtr(ptr), src); } template diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.h b/hphp/runtime/vm/translator/hopt/hhbctranslator.h index 1e5a4355a..c057f51a0 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.h +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.h @@ -564,6 +564,14 @@ private: const Func* getCurFunc() { return m_curFunc; } Class* getCurClass() { return getCurFunc()->cls(); } Unit* getCurUnit() { return getCurFunc()->unit(); } + + SrcKey getCurSrcKey() { return SrcKey(m_curFunc, m_bcOff); } + SrcKey getNextSrcKey() { + SrcKey srcKey(m_curFunc, m_bcOff); + srcKey.advance(m_curFunc->unit()); + return srcKey; + } + /* * Helpers for resolving bytecode immediate ids. */ diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 8b712c920..4635ae7ac 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -316,6 +316,7 @@ O(DecRef, ND, S(Gen), N|E|Mem|CRc|Refs|K) \ O(DecRefMem, ND, S(PtrToGen) \ C(Int), N|E|Mem|CRc|Refs) \ O(DecRefNZ, ND, S(Gen), Mem|CRc) \ +O(DecRefNZOrBranch, ND, S(Gen), Mem|CRc) \ O(DefLabel, DMulti, SUnk, E) \ O(Marker, ND, NA, E) \ O(DefFP, D(StkPtr), NA, E) \ diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index e12475c61..99d422c2c 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -130,6 +130,7 @@ SSATmp* Simplifier::simplify(IRInstruction* inst) { case PrintInt: case PrintBool: return simplifyPrint(inst); case DecRef: + case DecRefNZOrBranch: case DecRefNZ: return simplifyDecRef(inst); case IncRef: return simplifyIncRef(inst); case GuardType: return simplifyGuardType(inst); diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index edc7ff265..272753e9e 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -43,6 +43,7 @@ TraceBuilder::TraceBuilder(Offset initialBcOffset, , m_fpValue(nullptr) , m_spOffset(initialSpOffsetFromFp) , m_thisIsAvailable(false) + , m_refCountedMemValue(nullptr) , m_localValues(func->numLocals(), nullptr) , m_localTypes(func->numLocals(), Type::None) { @@ -160,6 +161,7 @@ SSATmp* TraceBuilder::genUnboxPtr(SSATmp* ptr) { */ bool TraceBuilder::isValueAvailable(SSATmp* tmp) const { while (true) { + if (m_refCountedMemValue == tmp) return true; if (anyLocalHasValue(tmp)) return true; IRInstruction* srcInstr = tmp->getInstruction(); @@ -1152,6 +1154,12 @@ SSATmp* TraceBuilder::genIterFree(uint32_t iterId) { void TraceBuilder::updateTrackedState(IRInstruction* inst) { Opcode opc = inst->getOpcode(); // Update tracked state of local values/types, stack/frame pointer, CSE, etc. + + // kill tracked memory values + if (inst->mayModifyRefs()) { + m_refCountedMemValue = nullptr; + } + switch (opc) { case Call: { m_spValue = inst->getDst(); @@ -1208,8 +1216,25 @@ void TraceBuilder::updateTrackedState(IRInstruction* inst) { break; } + case StProp: + case StPropNT: + // fall through to StMem; stored value is the same arg number (2) + case StMem: + case StMemNT: { + m_refCountedMemValue = inst->getSrc(2); + break; + } + + case LdMem: + case LdProp: + case LdRef: { + m_refCountedMemValue = inst->getDst(); + break; + } + case StRefNT: case StRef: { + m_refCountedMemValue = inst->getSrc(2); SSATmp* newRef = inst->getDst(); SSATmp* prevRef = inst->getSrc(0); // update other tracked locals that also contain prevRef @@ -1312,6 +1337,7 @@ void TraceBuilder::saveState(Block* block) { state->thisAvailable = m_thisIsAvailable; state->localValues = m_localValues; state->localTypes = m_localTypes; + state->refCountedMemValue = m_refCountedMemValue; m_snapshots[block] = state; } } @@ -1351,6 +1377,11 @@ void TraceBuilder::mergeState(State* state) { state->localTypes[i] = (t1 == Type::None || t2 == Type::None) ? Type::None : Type::unionOf(t1, t2); } + // Reference counted memory value is available only if it is available on both + // paths + if (state->refCountedMemValue != m_refCountedMemValue) { + state->refCountedMemValue = nullptr; + } } void TraceBuilder::useState(Block* block) { @@ -1361,6 +1392,7 @@ void TraceBuilder::useState(Block* block) { m_fpValue = state->fpValue; m_spOffset = state->spOffset; m_thisIsAvailable = state->thisAvailable; + m_refCountedMemValue = state->refCountedMemValue; m_localValues = std::move(state->localValues); m_localTypes = std::move(state->localTypes); delete state; @@ -1380,6 +1412,7 @@ void TraceBuilder::clearTrackedState() { m_spValue = m_fpValue = nullptr; m_spOffset = 0; m_thisIsAvailable = false; + m_refCountedMemValue = nullptr; for (auto i = m_snapshots.begin(), end = m_snapshots.end(); i != end; ++i) { delete *i; *i = nullptr; @@ -1572,8 +1605,9 @@ void TraceBuilder::optimizeTrace() { // Could have converted a conditional branch to Jmp; clear next. block->setNext(nullptr); } else { - // if the last instruction was a branch, we already saved state for the - // target in updateTrackedState(). Now save state for the fall-through path. + // if the last instruction was a branch, we already saved state + // for the target in updateTrackedState(). Now save state for + // the fall-through path. saveState(block->getNext()); } } diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.h b/hphp/runtime/vm/translator/hopt/tracebuilder.h index 367d9125b..db45ffebd 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.h +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.h @@ -357,6 +357,7 @@ private: bool thisAvailable; std::vector localValues; std::vector localTypes; + SSATmp* refCountedMemValue; }; void saveState(Block*); void mergeState(State* s1); @@ -414,14 +415,17 @@ private: */ SSATmp* m_spValue; // current physical sp SSATmp* m_fpValue; // current physical fp - int32_t m_spOffset; // offset of physical sp from physical fp + int32_t m_spOffset; // offset of physical sp from physical fp SSATmp* m_curFunc; // current function context CSEHash m_cseHash; bool m_thisIsAvailable; // true only if current ActRec has non-null this + // state of values in memory + SSATmp* m_refCountedMemValue; + // vectors that track local values & types std::vector m_localValues; - std::vector m_localTypes; + std::vector m_localTypes; }; template<>