Skip to content

Commit

Permalink
OS#19979778 : Merge uses of auxSlotPtrSym on dead edges as well
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Meghana Gupta committed Feb 21, 2019
1 parent 4a18b8a commit 8dc3939
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
35 changes: 33 additions & 2 deletions lib/Backend/BackwardPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block)
BVSparse<JitArenaAllocator> * typesNeedingKnownObjectLayout = nullptr;
BVSparse<JitArenaAllocator> * slotDeadStoreCandidates = nullptr;
BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed = nullptr;
BVSparse<JitArenaAllocator> * auxSlotPtrUpwardExposedUses = nullptr;
BVSparse<JitArenaAllocator> * couldRemoveNegZeroBailoutForDef = nullptr;
BVSparse<JitArenaAllocator> * liveFixedFields = nullptr;
#if DBG
Expand All @@ -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<JitArenaAllocator>, this->tempAlloc);
}

#if DBG
if (!IsCollectionPass() && this->DoMarkTempObjectVerify())
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -4267,6 +4297,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") },
};
Expand Down Expand Up @@ -5603,12 +5634,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)
{
Expand Down
2 changes: 2 additions & 0 deletions lib/Backend/FlowGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ class BasicBlock
BVSparse<JitArenaAllocator> * upwardExposedFields;
BVSparse<JitArenaAllocator> * typesNeedingKnownObjectLayout;
BVSparse<JitArenaAllocator> * slotDeadStoreCandidates;
BVSparse<JitArenaAllocator> * auxSlotPtrUpwardExposedUses;
TempNumberTracker * tempNumberTracker;
TempObjectTracker * tempObjectTracker;
#if DBG
Expand Down Expand Up @@ -437,6 +438,7 @@ class BasicBlock
upwardExposedFields(nullptr),
typesNeedingKnownObjectLayout(nullptr),
slotDeadStoreCandidates(nullptr),
auxSlotPtrUpwardExposedUses(nullptr),
tempNumberTracker(nullptr),
tempObjectTracker(nullptr),
#if DBG
Expand Down

0 comments on commit 8dc3939

Please sign in to comment.