[AMDGPU] SILowerSGPRSpills: do not update MRI reserve registers

[perlfu]

VGPRs used for spilling do not require explicit reservation with MRI. freezeReservedRegs() executed before register allocation ensures these are placed in the reserve set.

The only pass after SILowerSGPRSpills is SIPreAllocateWWMRegs which explicitly tests for interference before register allocation so should not reuse a WWM VGPR holding spill data. reserveReg prevents calculation of correct liveness for physical registers which could be used to extend SIPreAllocateWWMRegs.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Carl Ritson (perlfu)

Changes

VGPRs used for spilling do not require explicit reservation with MRI. freezeReservedRegs() executed before register allocation ensures these are placed in the reserve set.

The only pass after SILowerSGPRSpills is SIPreAllocateWWMRegs which explicitly tests for interference before register allocation so should not reuse a WWM VGPR holding spill data. reserveReg prevents calculation of correct liveness for physical registers which could be used to extend SIPreAllocateWWMRegs.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp (-9)
• (modified) llvm/test/CodeGen/AMDGPU/sgpr-spill-overlap-wwm-reserve.mir (+1-1)

diff --git a/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp b/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp
index 0ba7792ac436d4..70ffb8ea0a6220 100644
--- a/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp
@@ -332,7 +332,6 @@ bool SILowerSGPRSpills::runOnMachineFunction(MachineFunction &MF) {
   }
 
   bool MadeChange = false;
-  bool NewReservedRegs = false;
   bool SpilledToVirtVGPRLanes = false;
 
   // TODO: CSR VGPRs will never be spilled to AGPRs. These can probably be
@@ -370,7 +369,6 @@ bool SILowerSGPRSpills::runOnMachineFunction(MachineFunction &MF) {
           // intermediate spills is implemented. There is no such support
           // currently exist in the LLVM compiler.
           if (FuncInfo->allocateSGPRSpillToVGPRLane(MF, FI, true)) {
-            NewReservedRegs = true;
             bool Spilled = TRI->eliminateSGPRToVGPRSpillFrameIndex(
                 MI, FI, nullptr, Indexes, LIS, true);
             if (!Spilled)
@@ -442,12 +440,5 @@ bool SILowerSGPRSpills::runOnMachineFunction(MachineFunction &MF) {
   SaveBlocks.clear();
   RestoreBlocks.clear();
 
-  // Updated the reserved registers with any physical VGPRs added for SGPR
-  // spills.
-  if (NewReservedRegs) {
-    for (Register Reg : FuncInfo->getWWMReservedRegs())
-      MRI.reserveReg(Reg, TRI);
-  }
-
   return MadeChange;
 }
diff --git a/llvm/test/CodeGen/AMDGPU/sgpr-spill-overlap-wwm-reserve.mir b/llvm/test/CodeGen/AMDGPU/sgpr-spill-overlap-wwm-reserve.mir
index 1473e667f894cd..4544b177351eeb 100644
--- a/llvm/test/CodeGen/AMDGPU/sgpr-spill-overlap-wwm-reserve.mir
+++ b/llvm/test/CodeGen/AMDGPU/sgpr-spill-overlap-wwm-reserve.mir
@@ -1,5 +1,5 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
-# RUN: llc -mtriple=amdgcn-unknown-amdpal -mcpu=gfx1030 -start-before=si-lower-sgpr-spills -stop-after=prologepilog -verify-machineinstrs --stress-regalloc=2 -o - %s | FileCheck -check-prefix GCN %s
+# RUN: llc -mtriple=amdgcn-unknown-amdpal -mcpu=gfx1030 -start-before=si-lower-sgpr-spills -stop-after=prologepilog -verify-machineinstrs --stress-regalloc=5 -o - %s | FileCheck -check-prefix GCN %s
 
 --- |
   define amdgpu_gfx [13 x i32] @test_main() #0 {

[perlfu]

To explain this test change:
--stress-regalloc=2 limits the VGPR class to 2 registers after reserve registers are subtracted.
Without reservation by SILowerSGPRSpills the 3 VGPRs count against the VGPR class usage, so the class size must be enlarged (to 5) to accommodate them.

[perlfu]

To preempt an obvious follow up question: "how does SILowerSGPRSpill exceed the register limit?"
allocateSGPRSpillToVGPRLane uses findUnusedRegister which does not consult the allocation in RegisterClassInfo which imposes the limit.

[cdevadas]

This option should have been better integrated into MRI to fully limit the register usage in various other passes.

[cdevadas]

The phys VGPRs used for CSR SGPR spilling shouldn't be reused during SIPreAllocateWWMRegs.
I hope they will be marked reserved while getting the AllocationOrder from RegClassInfo.runOnMachineFunction(MF).
@arsenm any specific reason they are marked Reserved here explicitly?

[arsenm]

This shouldn't be necessary in the physical register CSR case