Mark propagated values as global in post RA spill cleanup

Post RA spill cleanup pass creates new virtual variables and ties them
to appropriate physical registers. If data flow is run after this pass,
it can lead to missing links because of new virtual variables. As a fix,
any time we break such a virtual variable link, we mark it as global so
that later passes don't assume it's local and try to optimize it.

Author: pratikashar

visa/SpillFillPropagation.cpp

@@ -23,6 +23,7 @@ SpillFillPropagation::SpillFillPropagation(G4_Kernel &k, IR_Builder &b,
 void SpillFillPropagation::clearTable() {
   offsetToGRFs.clear();
   grfToOffsets.clear();
+  grfToDeclare.clear();
 }
 
 // Record that a physical GRF holds the value for a scratch offset.
@@ -50,6 +51,8 @@ void SpillFillPropagation::invalidateGRF(unsigned grfNum) {
     }
     grfToOffsets.erase(it);
   }
+
+  grfToDeclare[grfNum].clear();
 }
 
 // Remove all physical GRFs mapping to given offset
@@ -141,9 +144,9 @@ bool SpillFillPropagation::replaceFillWithMovs(
       continue;
     }
 
-    bool canDoTwoRows =
-        (i + 1 < numRows) && (srcGRFs[i + 1] != dstStartGRF + i + 1) &&
-        (srcGRFs[i] + 1 == srcGRFs[i + 1]);
+    bool canDoTwoRows = (i + 1 < numRows) &&
+                        (srcGRFs[i + 1] != dstStartGRF + i + 1) &&
+                        (srcGRFs[i] + 1 == srcGRFs[i + 1]);
 
     unsigned rowsThisMov = canDoTwoRows ? 2 : 1;
     G4_ExecSize execSize =
@@ -158,16 +161,12 @@ bool SpillFillPropagation::replaceFillWithMovs(
         name, G4_GRF, kernel.numEltPerGRF<Type_UD>(), rowsThisMov, Type_UD);
     srcDcl->getRegVar()->setPhyReg(builder.phyregpool.getGreg(srcGRFs[i]), 0);
 
-    G4_SrcRegRegion *src = builder.createSrc(
-        srcDcl->getRegVar(), 0, 0, builder.getRegionStride1(), type);
+    G4_SrcRegRegion *src = builder.createSrc(srcDcl->getRegVar(), 0, 0,
+                                             builder.getRegionStride1(), type);
 
     G4_INST *mov =
         builder.createMov(execSize, dst, src, InstOpt_WriteEnable, false);
-    // This mov's dst may be propagated to successor BB. So mark it as global
-    // here. Since this is post RA pass, it doesn't reuse old virtual variables.
-    // Therefore if data is run again in later passes, it could incorrectly
-    // treat mov dst as a local otherwise (and remove it in some cases).
-    dstTopDcl->setForceGlobal();
+
     mov->setVISAId(fill->getVISAId());
 
     bb->insertBefore(fillIt, mov);
@@ -179,11 +178,26 @@ bool SpillFillPropagation::replaceFillWithMovs(
     i += rowsThisMov;
   }
 
+  markGlobalDcls(srcGRFs);
+
   // Erase the fill intrinsic.
   fillIt = bb->erase(fillIt);
   return true;
 }
 
+void SpillFillPropagation::markGlobalDcls(
+    const std::vector<unsigned int> &GRFs) {
+  // Mark all G4_Declare* containing each GRF as global so they're preserved
+  for (auto GRF : GRFs) {
+    auto it = grfToDeclare.find(GRF);
+    if (it == grfToDeclare.end())
+      continue;
+    for (auto dcl : it->second) {
+      dcl->setForceGlobal();
+    }
+  }
+}
+
 // Replace a pending fill with GRF-to-GRF movs inserted after the source
 // instruction pointed to by rit (backward pass). Erases the pending fill
 // instruction and re-seats rit. Returns false if src/dst overlap unsafely.
@@ -245,6 +259,7 @@ bool SpillFillPropagation::replaceFillWithMovsAfter(
     i += rowsThisMov;
   }
 
+  pf.dstTopDcl->setForceGlobal();
 
   // Erase the pending fill instruction.
   bb->erase(pf.instIt);
@@ -297,13 +312,21 @@ static bool hasIndirectOperand(G4_INST *inst) {
   return false;
 }
 
+void SpillFillPropagation::mapGRFsToDcl(unsigned int startGRF,
+                                        unsigned int numGRFs, G4_Declare *dcl) {
+  for (unsigned grf = startGRF; grf != (startGRF + numGRFs); ++grf) {
+    grfToDeclare[grf].insert(dcl);
+  }
+}
+
 // Forward (top-down) pass: track which GRFs hold spilled data and replace
 // fills with movs when the data is already available in a GRF.
 void SpillFillPropagation::processBBForward(G4_BB *bb) {
   // Load carried-over state or start fresh.
   auto entryIt = bbEntryState.find(bb);
   if (entryIt != bbEntryState.end()) {
-    offsetToGRFs = std::move(entryIt->second);
+    offsetToGRFs = std::move(entryIt->second.offsetToGRFs);
+    grfToDeclare = std::move(entryIt->second.grfToDeclare);
     // Rebuild the reverse map.
     grfToOffsets.clear();
     for (auto &[off, grfs] : offsetToGRFs)
@@ -327,6 +350,8 @@ void SpillFillPropagation::processBBForward(G4_BB *bb) {
             getGRFRange(spill->getPayload());
         for (unsigned i = 0; i < numRows; ++i)
           addEntry(offset + i, payloadStartGRF + i, /*clearOld=*/true);
+        mapGRFsToDcl(payloadStartGRF, numRows,
+                     spill->getPayload()->getTopDcl());
       } else {
         // Scatter spill or non-WriteEnable: invalidate any GRF mapping to
         // these offsets since we cannot propagate through them.
@@ -350,13 +375,15 @@ void SpillFillPropagation::processBBForward(G4_BB *bb) {
           invalidateClobberedEntries(inst);
           for (unsigned i = 0; i < numRows; ++i)
             addEntry(offset + i, dstStartGRF + i);
+          mapGRFsToDcl(dstStartGRF, numRows, fill->getDst()->getTopDcl());
           // it already advanced by erase
           continue;
         }
         // Fill not replaced — invalidate dst GRFs and record.
         invalidateClobberedEntries(inst);
         for (unsigned i = 0; i < numRows; ++i)
           addEntry(offset + i, dstStartGRF + i);
+        mapGRFsToDcl(dstStartGRF, numRows, fill->getDst()->getTopDcl());
       } else {
         // Non-WriteEnable fill: invalidate destination GRFs (WAW clobber).
         invalidateClobberedEntries(inst);
@@ -395,8 +422,12 @@ void SpillFillPropagation::processBBForward(G4_BB *bb) {
     G4_INST *lastInst = bb->back();
     if (lastInst->opcode() == G4_goto && lastInst->getPredicate()) {
       G4_BB *ftBB = bb->Succs.empty() ? nullptr : bb->Succs.front();
-      if (ftBB && ftBB->Preds.size() == 1 && ftBB->Preds.front() == bb)
-        bbEntryState[ftBB] = offsetToGRFs;
+      if (ftBB && ftBB->Preds.size() == 1 && ftBB->Preds.front() == bb) {
+        BBEntryState state;
+        state.offsetToGRFs = offsetToGRFs;
+        state.grfToDeclare = grfToDeclare;
+        bbEntryState[ftBB] = state;
+      }
     }
   }
 }
@@ -552,6 +583,7 @@ void SpillFillPropagation::processBBBackward(G4_BB *bb) {
 
           if (replaceFillWithMovsAfter(bb, rit, insertAfterIt, *matchedPF,
                                        srcGRFs)) {
+            fill->getDst()->getTopDcl()->setForceGlobal();
             for (unsigned g = matchedPF->dstStartGRF;
                  g <= matchedPF->dstEndGRF; ++g)
               clobberedRows.insert(g);
@@ -641,6 +673,8 @@ void SpillFillPropagation::processBBBackward(G4_BB *bb) {
 
           if (replaceFillWithMovsAfter(bb, rit, insertAfterIt, *matchedPF,
                                        srcGRFs)) {
+            auto *payloadDcl = spill->getPayload()->getTopDcl();
+            payloadDcl->setForceGlobal();
             for (unsigned g = matchedPF->dstStartGRF;
                  g <= matchedPF->dstEndGRF; ++g)
               clobberedRows.insert(g);

visa/SpillFillPropagation.h

@@ -30,6 +30,11 @@ struct PendingFill {
   unsigned dstEndGRF; // inclusive
 };
 
+typedef struct BBEntryState {
+  std::unordered_map<unsigned, std::set<unsigned>> offsetToGRFs;
+  std::unordered_map<unsigned, std::set<G4_Declare *>> grfToDeclare;
+} BBEntryState;
+
 // Post-RA pass that eliminates redundant fill intrinsics by tracking which
 // physical GRFs still hold spilled/filled data and replacing fills with
 // GRF-to-GRF mov instructions when possible.
@@ -46,13 +51,13 @@ class SpillFillPropagation {
   std::unordered_map<unsigned, std::set<unsigned>> offsetToGRFs;
   // Reverse map: physical GRF number -> set of scratch offsets it maps to
   std::unordered_map<unsigned, std::set<unsigned>> grfToOffsets;
+  // Map GRF -> set of G4_Declare* to mark latter as global
+  std::unordered_map<unsigned, std::set<G4_Declare *>> grfToDeclare;
   // Per-BB entry state for fall-through propagation
-  std::unordered_map<G4_BB *, std::unordered_map<unsigned, std::set<unsigned>>>
-      bbEntryState;
+  std::unordered_map<G4_BB *, BBEntryState> bbEntryState;
 
   void clearTable();
-  void addEntry(unsigned scratchOffset, unsigned grfNum,
-                bool clearOld = false);
+  void addEntry(unsigned scratchOffset, unsigned grfNum, bool clearOld = false);
   void invalidateGRF(unsigned grfNum);
   void invalidateOffset(unsigned offset);
   std::pair<unsigned, unsigned> getGRFRange(G4_Operand *opnd);
@@ -61,19 +66,20 @@ class SpillFillPropagation {
                            G4_FillIntrinsic *fill,
                            const std::vector<unsigned> &srcGRFs);
   void invalidateClobberedEntries(G4_INST *inst);
-  void invalidateDst(
-      G4_INST *inst,
-      std::unordered_map<unsigned, PendingFill> &pendingFills);
-  void invalidateSrcs(
-      G4_INST *inst,
-      std::unordered_map<unsigned, PendingFill> &pendingFills);
+  void invalidateDst(G4_INST *inst,
+                     std::unordered_map<unsigned, PendingFill> &pendingFills);
+  void invalidateSrcs(G4_INST *inst,
+                      std::unordered_map<unsigned, PendingFill> &pendingFills);
   void processBBForward(G4_BB *bb);
   void processBBBackward(G4_BB *bb);
   bool replaceFillWithMovsAfter(G4_BB *bb, INST_LIST_RITER &rit,
                                 INST_LIST_ITER &insertAfterIt,
                                 const PendingFill &pf,
                                 const std::vector<unsigned> &srcGRFs);
   bool hasAssignedGRF(G4_Declare *topdcl) const;
+  void markGlobalDcls(const std::vector<unsigned int> &GRFs);
+  void mapGRFsToDcl(unsigned int startGRF, unsigned int numGRFs,
+                    G4_Declare *dcl);
 
 public:
   SpillFillPropagation(G4_Kernel &k, IR_Builder &b, GlobalRA &g);