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.
chakra-core · Feb 21, 2019 · 8dc3939 · 8dc3939
1 parent 4a18b8a
commit 8dc3939
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 2 deletions.
diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp
@@ -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
@@ -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())
     {
@@ -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;
@@ -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") },
     };
@@ -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)
         {

diff --git a/lib/Backend/FlowGraph.h b/lib/Backend/FlowGraph.h
@@ -391,6 +391,7 @@ class BasicBlock
     BVSparse<JitArenaAllocator> *              upwardExposedFields;
     BVSparse<JitArenaAllocator> *              typesNeedingKnownObjectLayout;
     BVSparse<JitArenaAllocator> *              slotDeadStoreCandidates;
+    BVSparse<JitArenaAllocator> *              auxSlotPtrUpwardExposedUses; 
     TempNumberTracker *                     tempNumberTracker;
     TempObjectTracker *                     tempObjectTracker;
 #if DBG
@@ -437,6 +438,7 @@ class BasicBlock
         upwardExposedFields(nullptr),
         typesNeedingKnownObjectLayout(nullptr),
         slotDeadStoreCandidates(nullptr),
+        auxSlotPtrUpwardExposedUses(nullptr),
         tempNumberTracker(nullptr),
         tempObjectTracker(nullptr),
 #if DBG