[llvm] [InlineSpiller] Avoid spill hoisting interference (PR #211809)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-amdgpu

Author: Lukas Sommer (sommerlukas)

<details>
<summary>Changes</summary>

When hoisting spills and the source is a PHI, the "hoisted" spill may be inserted only after the basic block prologue. This may require extending this segment of the source value's live interval.

Abort the hoisting if this extension of the segment would interfere with another live interval assigned to the same physical register.

---
Full diff: https://github.com/llvm/llvm-project/pull/211809.diff


2 Files Affected:

- (modified) llvm/lib/CodeGen/InlineSpiller.cpp (+28-10) 
- (added) llvm/test/CodeGen/AMDGPU/regalloc-hoist-spill-live-range-interference.mir (+45) 


``````````diff
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index e03b4f4b4046c..f3682a2e24808 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -454,6 +454,34 @@ bool InlineSpiller::hoistSpillInsideBB(LiveInterval &SpillLI,
   if (DefMBB != CopyMI.getParent() || !SrcQ.isKill())
     return false;
 
+  MachineBasicBlock *MBB = DefMBB;
+  MachineBasicBlock::iterator MII;
+  if (SrcVNI->isPHIDef())
+    MII = MBB->SkipPHIsLabelsAndDebug(MBB->begin(), SrcReg);
+  else {
+    MachineInstr *DefMI = LIS.getInstructionFromIndex(SrcVNI->def);
+    assert(DefMI && "Defining instruction disappeared");
+    MII = DefMI;
+    ++MII;
+  }
+
+  // When the def is a PHI, the store may be inserted after the prologue
+  // instructions. In that case, the segment may need to be extended to the
+  // store (see below). Do not hoist if there is an interference between the end
+  // of the segment and the insertion point.
+  if (SrcVNI->isPHIDef() && Matrix && VRM.hasPhys(SrcReg)) {
+    // Here, MII points to the instruction before which the store will be
+    // inserted. Using that instruction's base index is a safe upper bound for
+    // the interference check.
+    SlotIndex InsertIdx = MII == MBB->end()
+                              ? LIS.getMBBEndIdx(MBB)
+                              : LIS.getInstructionIndex(*MII).getBaseIndex();
+    if (SrcQ.endPoint() < InsertIdx &&
+        Matrix->checkInterference(SrcQ.endPoint(), InsertIdx,
+                                  VRM.getPhys(SrcReg)))
+      return false;
+  }
+
   // Conservatively extend the stack slot range to the range of the original
   // value. We may be able to do better with stack slot coloring by being more
   // careful here.
@@ -468,16 +496,6 @@ bool InlineSpiller::hoistSpillInsideBB(LiveInterval &SpillLI,
   // any later spills of the same value.
   eliminateRedundantSpills(SrcLI, SrcVNI);
 
-  MachineBasicBlock *MBB = LIS.getMBBFromIndex(SrcVNI->def);
-  MachineBasicBlock::iterator MII;
-  if (SrcVNI->isPHIDef())
-    MII = MBB->SkipPHIsLabelsAndDebug(MBB->begin(), SrcReg);
-  else {
-    MachineInstr *DefMI = LIS.getInstructionFromIndex(SrcVNI->def);
-    assert(DefMI && "Defining instruction disappeared");
-    MII = DefMI;
-    ++MII;
-  }
   MachineInstrSpan MIS(MII, MBB);
   // Insert spill without kill flag immediately after def.
   TII.storeRegToStackSlot(*MBB, MII, SrcReg, false, StackSlot,
diff --git a/llvm/test/CodeGen/AMDGPU/regalloc-hoist-spill-live-range-interference.mir b/llvm/test/CodeGen/AMDGPU/regalloc-hoist-spill-live-range-interference.mir
new file mode 100644
index 0000000000000..9b7411d98dc75
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/regalloc-hoist-spill-live-range-interference.mir
@@ -0,0 +1,45 @@
+# RUN: llc -mtriple=amdgpu9.50-amd-amdhsa -passes='greedy<wwm>' \
+# RUN:   -stress-regalloc=3 -verify-regalloc -o - %s | FileCheck %s
+#
+# InlineSpiller tries to hoist the spill of %1 by storing its sibling %0 after
+# the basic block prologue. Extending the PHI-defined %0 to that insertion point
+# would overlap an interval already assigned to the same physical register.
+# Reject the hoist and spill %0 at the original copy instead.
+
+# CHECK-LABEL: name: hoist_spill_live_range_interference
+# CHECK: bb.2:
+# CHECK: SI_SPILL_WWM_V32_SAVE %0
+# CHECK-NEXT: $exec = S_AND_B64
+
+---
+name:            hoist_spill_live_range_interference
+tracksRegLiveness: true
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+registers:
+  - { id: 0, class: vgpr_32, flags: [ WWM_REG ], split-from: '%2' }
+  - { id: 1, class: vgpr_32, flags: [ WWM_REG ], split-from: '%2' }
+  - { id: 2, class: vgpr_32, flags: [ WWM_REG ] }
+  - { id: 3, class: vgpr_32, flags: [ WWM_REG ] }
+  - { id: 4, class: vgpr_32, flags: [ WWM_REG ] }
+body: |
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $scc
+    %2:vgpr_32 = KILL
+    %0:vgpr_32 = COPY %2
+    S_CBRANCH_SCC1 %bb.2, implicit $scc
+
+  bb.1:
+    successors: %bb.2
+    %0:vgpr_32 = COPY %2
+
+  bb.2:
+    %1:vgpr_32 = lr-split WWM_COPY %0:vgpr_32
+    $exec = S_AND_B64 $exec, $exec, implicit-def $scc
+    %3:vgpr_32 = IMPLICIT_DEF
+    %4:vgpr_32 = IMPLICIT_DEF
+    KILL %2:vgpr_32, %3:vgpr_32, %4:vgpr_32
+    KILL %1:vgpr_32, %2:vgpr_32
+    S_ENDPGM 0
+...

``````````

</details>


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