From a3b73becf22726690d54e27ba57b6c91ab467c1c Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Mon, 11 Feb 2019 18:30:53 -0800 Subject: [PATCH] OS#19979778 : Merge uses of auxSlotPtrSym on dead edges as well This is a fix for use-before-def of an auxSlotPtrSym. When there is a LdFld which is a candidate of ReuseAuxSlotSymPtr optimization inside the loop which has no backedge (all are dead), the DeadStore phase never marks SetProducesAuxSlotPtr on the opnd. This is because we don't merge upwardExposedUses on dead edges, and so the optimization never sees a set bit of the auxSlotPtrSym in upwardExposedUses in the 2nd pass. Ideally, if GlobOpt reset block->loopbfor such loops after GlobOpt::RemoveCodeAfterNoFallThroughInstr we wouldn't be seeing this. With this change we have a new bit vector called auxSlotPtrUpwardExposedUses which we merge on even dead successor edges to fix this issue. --- lib/Backend/BackwardPass.cpp | 35 +++++++++++++++++++++++++++++++++-- lib/Backend/FlowGraph.h | 2 ++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp index d6677c5dc7d..941a3e6a10e 100644 --- a/lib/Backend/BackwardPass.cpp +++ b/lib/Backend/BackwardPass.cpp @@ -486,6 +486,7 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block) BVSparse * typesNeedingKnownObjectLayout = nullptr; BVSparse * slotDeadStoreCandidates = nullptr; BVSparse * byteCodeUpwardExposedUsed = nullptr; + BVSparse * auxSlotPtrUpwardExposedUses = nullptr; BVSparse * couldRemoveNegZeroBailoutForDef = nullptr; BVSparse * liveFixedFields = nullptr; #if DBG @@ -505,6 +506,11 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block) #endif } + if ((this->tag == Js::DeadStorePhase) && !PHASE_OFF(Js::ReuseAuxSlotPtrPhase, this->func)) + { + auxSlotPtrUpwardExposedUses = JitAnew(this->tempAlloc, BVSparse, this->tempAlloc); + } + #if DBG if (!IsCollectionPass() && this->DoMarkTempObjectVerify()) { @@ -1124,6 +1130,29 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block) NEXT_DEAD_SUCCESSOR_BLOCK; } + if (auxSlotPtrUpwardExposedUses) + { + FOREACH_SUCCESSOR_BLOCK(blockSucc, block) + { + Assert(blockSucc->auxSlotPtrUpwardExposedUses || blockSucc->isLoopHeader); + if (blockSucc->auxSlotPtrUpwardExposedUses) + { + auxSlotPtrUpwardExposedUses->Or(blockSucc->auxSlotPtrUpwardExposedUses); + } + } + NEXT_SUCCESSOR_BLOCK; + + FOREACH_DEAD_SUCCESSOR_BLOCK(deadBlockSucc, block) + { + Assert(deadBlockSucc->auxSlotPtrUpwardExposedUses || deadBlockSucc->isLoopHeader); + if (deadBlockSucc->auxSlotPtrUpwardExposedUses) + { + auxSlotPtrUpwardExposedUses->Or(deadBlockSucc->auxSlotPtrUpwardExposedUses); + } + } + NEXT_DEAD_SUCCESSOR_BLOCK; + } + if (block->isLoopHeader) { this->DeleteBlockData(block); @@ -1187,6 +1216,7 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block) block->upwardExposedFields = upwardExposedFields; block->typesNeedingKnownObjectLayout = typesNeedingKnownObjectLayout; block->byteCodeUpwardExposedUsed = byteCodeUpwardExposedUsed; + block->auxSlotPtrUpwardExposedUses = auxSlotPtrUpwardExposedUses; #if DBG block->byteCodeRestoreSyms = byteCodeRestoreSyms; block->excludeByteCodeUpwardExposedTracking = excludeByteCodeUpwardExposedTracking; @@ -4275,6 +4305,7 @@ BackwardPass::DumpBlockData(BasicBlock * block, IR::Instr* instr) { block->typesNeedingKnownObjectLayout, _u("Needs Known Object Layout") }, { block->upwardExposedFields, _u("Exposed Fields") }, { block->byteCodeUpwardExposedUsed, _u("Byte Code Use") }, + { block->auxSlotPtrUpwardExposedUses, _u("Aux Slot Use")}, { byteCodeRegisterUpwardExposed, _u("Byte Code Reg Use") }, { !this->IsCollectionPass() && !block->isDead && this->DoDeadStoreSlots() ? block->slotDeadStoreCandidates : nullptr, _u("Slot deadStore candidates") }, }; @@ -5611,12 +5642,12 @@ BackwardPass::TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock * { // This is an upward-exposed use of the aux slot pointer. Assert(auxSlotPtrSym); - auxSlotPtrUpwardExposed = this->currentBlock->upwardExposedUses->TestAndSet(auxSlotPtrSym->m_id); + auxSlotPtrUpwardExposed = this->currentBlock->auxSlotPtrUpwardExposedUses->TestAndSet(auxSlotPtrSym->m_id); } else if (auxSlotPtrSym != nullptr) { // The aux slot pointer is not upward-exposed at this point. - auxSlotPtrUpwardExposed = this->currentBlock->upwardExposedUses->TestAndClear(auxSlotPtrSym->m_id); + auxSlotPtrUpwardExposed = this->currentBlock->auxSlotPtrUpwardExposedUses->TestAndClear(auxSlotPtrSym->m_id); } if (!this->IsPrePass() && auxSlotPtrUpwardExposed) { diff --git a/lib/Backend/FlowGraph.h b/lib/Backend/FlowGraph.h index 87c1f8e8c2a..59d5ba58eec 100644 --- a/lib/Backend/FlowGraph.h +++ b/lib/Backend/FlowGraph.h @@ -388,6 +388,7 @@ class BasicBlock BVSparse * upwardExposedFields; BVSparse * typesNeedingKnownObjectLayout; BVSparse * slotDeadStoreCandidates; + BVSparse * auxSlotPtrUpwardExposedUses; TempNumberTracker * tempNumberTracker; TempObjectTracker * tempObjectTracker; #if DBG @@ -434,6 +435,7 @@ class BasicBlock upwardExposedFields(nullptr), typesNeedingKnownObjectLayout(nullptr), slotDeadStoreCandidates(nullptr), + auxSlotPtrUpwardExposedUses(nullptr), tempNumberTracker(nullptr), tempObjectTracker(nullptr), #if DBG