[llvm] [1/2][AMDGPU] Fixed crash due to virtual register defs not dominating uses (PR #198472)

[via llvm-commits]

================
@@ -476,6 +492,69 @@ void AMDGPURewriteAGPRCopyMFMAImpl::collectSpillIndexUses(
   }
 }
 
+bool AMDGPURewriteAGPRCopyMFMAImpl::insertImplicitDefsForLiveSegments(
+    LiveInterval &StackLI, int Slot, ArrayRef<MachineInstr *> SpillStores,
+    Register NewVReg) const {
+  const unsigned ScanLimit = ImplicitDefScanLimit;
+
+  SmallPtrSet<MachineInstr *, 4> StoreSet(SpillStores.begin(),
+                                          SpillStores.end());
+
+  SmallVector<MachineInstr *, 2> InsertedDefs;
+
+  for (const LiveRange::Segment &Seg : StackLI) {
+    MachineInstr *MI = LIS.getInstructionFromIndex(Seg.start);
+    // Fall back to LIS query in case the segment start does not correspond
+    // to an instruction.
+    MachineBasicBlock *MBB =
+        MI ? MI->getParent() : LIS.getMBBFromIndex(Seg.start);
+
+    // Bail-out: we could not resolve any MBB for this segment start.
+    // Roll back any defs already inserted for earlier segments -- the
+    // caller must skip unspilling this slot entirely.
+    if (!MBB) {
+      for (MachineInstr *Def : InsertedDefs) {
+        LIS.RemoveMachineInstrFromMaps(*Def);
+        Def->eraseFromParent();
+      }
+      return false;
+    }
+
+    // Scan forward from the segment start looking for a spill store to this
+    // slot. LiveStacks liveness for a store can start a few instructions
+    // before the store itself, so we allow a small window controlled by
+    // ImplicitDefScanLimit. Redundant IMPLICIT_DEFs (e.g. because of a small
+    // scan limit) are harmless, since they will be cleaned up by downstream
+    // transformations.
+    MachineBasicBlock::iterator It = MI ? MI->getIterator() : MBB->begin();
+    bool FoundStore = false;
+    for (unsigned I = 0; I < ScanLimit && It != MBB->end(); ++I, ++It) {
+      if (StoreSet.count(&*It)) {
+        FoundStore = true;
+        break;
+      }
+    }
----------------
ruiling wrote:

Hi @arsenm, I guess I see your point. Your point is the LiveInterval needs to be reconstructed as pseudoSSA value numbering. My point is how to insert implicit_def to achieve it. Looking at the liveStack of the problematic stack slot: `SS#2 [2712r,4016B:0)[4240r,4576B:0)[16560r,24464r:0) 0 at x  weight:0.000000e+00 [VReg_512_Align2]`. The first entry `[2712r,4016B:0)` points to the old `implicit_def` for the spilled register. The live stack has inherited all the live segments from the spilled register just without the value-numbering. If we can assume the LiveStack would inherit the segments from LiveInterval like the case here. We only need to insert implicit_def before the starting point if it does not point to stack store to that slot.

https://github.com/llvm/llvm-project/pull/198472