[llvm-branch-commits] [llvm] AMDGPU: Delete spills of undef values (PR #119684)

Matt Arsenault via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Dec 12 17:01:59 PST 2024


https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/119684

>From c56e320b410a50791b006dd75c48ff74c89a735d Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Mon, 28 Mar 2022 11:24:48 -0400
Subject: [PATCH 1/2] AMDGPU: Delete spills of undef values

It would be a bit more logical to preserve the undef and do the normal
expansion, but this is less work. This avoids verifier errors in a
future patch which starts deleting liveness from registers after
allocation failures which results in spills of undef values.

https://reviews.llvm.org/D122607
---
 llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp     | 12 ++++++
 .../AMDGPU/sgpr-spill-partially-undef.mir     | 42 +++++++++++++++++++
 .../AMDGPU/spill-agpr-partially-undef.mir     | 34 +++++++++++++++
 llvm/test/CodeGen/AMDGPU/vgpr-spill.mir       | 34 +++++++++++++++
 4 files changed, 122 insertions(+)

diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
index 296c32fa4e0d09..4f8c5c6756b3bb 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
@@ -1954,6 +1954,13 @@ bool SIRegisterInfo::spillSGPR(MachineBasicBlock::iterator MI, int Index,
                                RegScavenger *RS, SlotIndexes *Indexes,
                                LiveIntervals *LIS, bool OnlyToVGPR,
                                bool SpillToPhysVGPRLane) const {
+  if (MI->getOperand(0).isUndef()) {
+    if (Indexes)
+      Indexes->removeMachineInstrFromMaps(*MI);
+    MI->eraseFromParent();
+    return true;
+  }
+
   SGPRSpillBuilder SB(*this, *ST.getInstrInfo(), isWave32, MI, Index, RS);
 
   ArrayRef<SpilledReg> VGPRSpills =
@@ -2375,6 +2382,11 @@ bool SIRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator MI,
     case AMDGPU::SI_SPILL_WWM_AV32_SAVE: {
       const MachineOperand *VData = TII->getNamedOperand(*MI,
                                                          AMDGPU::OpName::vdata);
+      if (VData->isUndef()) {
+        MI->eraseFromParent();
+        return true;
+      }
+
       assert(TII->getNamedOperand(*MI, AMDGPU::OpName::soffset)->getReg() ==
              MFI->getStackPtrOffsetReg());
 
diff --git a/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir b/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir
index 774785fb3966fc..d352e8a13da9f1 100644
--- a/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir
+++ b/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir
@@ -54,3 +54,45 @@ body:             |
     SI_SPILL_S64_SAVE renamable $sgpr4_sgpr5, %stack.0, implicit $exec, implicit $sgpr96_sgpr97_sgpr98_sgpr99, implicit $sgpr32 :: (store (s64) into %stack.0, align 4, addrspace 5)
 
 ...
+
+---
+name:  sgpr_spill_s32_undef
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+  hasSpilledSGPRs:     true
+  scratchRSrcReg:  '$sgpr96_sgpr97_sgpr98_sgpr99'
+  stackPtrOffsetReg: '$sgpr32'
+stack:
+  - { id: 0, type: spill-slot, size: 4, alignment: 4, stack-id: sgpr-spill }
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: sgpr_spill_s32_undef
+    ; CHECK: body:
+    ; CHECK-NEXT: bb.0:
+    ; CHECK-NOT: {{.+}}
+    ; CHECK: ...
+    SI_SPILL_S32_SAVE undef $sgpr8, %stack.0, implicit $exec, implicit $sgpr96_sgpr97_sgpr98_sgpr99, implicit $sgpr32 :: (store (s32) into %stack.0, align 4, addrspace 5)
+
+...
+
+---
+name:  sgpr_spill_s64_undef
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+  hasSpilledSGPRs:     true
+  scratchRSrcReg:  '$sgpr96_sgpr97_sgpr98_sgpr99'
+  stackPtrOffsetReg: '$sgpr32'
+stack:
+  - { id: 0, type: spill-slot, size: 8, alignment: 4, stack-id: sgpr-spill }
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: sgpr_spill_s64_undef
+    ; CHECK: body:
+    ; CHECK-NEXT: bb.0:
+    ; CHECK-NOT: {{.+}}
+    ; CHECK: ...
+    SI_SPILL_S64_SAVE undef $sgpr8_sgpr9, %stack.0, implicit $exec, implicit $sgpr96_sgpr97_sgpr98_sgpr99, implicit $sgpr32 :: (store (s64) into %stack.0, align 4, addrspace 5)
+
+...
diff --git a/llvm/test/CodeGen/AMDGPU/spill-agpr-partially-undef.mir b/llvm/test/CodeGen/AMDGPU/spill-agpr-partially-undef.mir
index c825674de7652c..b02b6e79d7a76f 100644
--- a/llvm/test/CodeGen/AMDGPU/spill-agpr-partially-undef.mir
+++ b/llvm/test/CodeGen/AMDGPU/spill-agpr-partially-undef.mir
@@ -71,3 +71,37 @@ body:             |
     ; CHECK-NEXT: BUFFER_STORE_DWORD_OFFSET killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr32, 4, 0, 0, implicit $exec, implicit killed $agpr0_agpr1 :: (store (s32) into %stack.0 + 4, addrspace 5)
     SI_SPILL_A64_SAVE killed $agpr0_agpr1, %stack.0, $sgpr32, 0, implicit $exec :: (store (s64) into %stack.0, addrspace 5)
 ...
+
+---
+name: spill_a32_undef
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, size: 4, alignment: 4 }
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  stackPtrOffsetReg: '$sgpr32'
+  frameOffsetReg: '$sgpr33'
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: spill_a32_undef
+    ; CHECK: S_ENDPGM 0
+    SI_SPILL_A32_SAVE undef $agpr0, %stack.0, $sgpr32, 0, implicit $exec :: (store (s32) into %stack.0, addrspace 5)
+    S_ENDPGM 0
+...
+
+---
+name: spill_a64_undef
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, size: 8, alignment: 4 }
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  stackPtrOffsetReg: '$sgpr32'
+  frameOffsetReg: '$sgpr33'
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: spill_a64_undef
+    ; CHECK: S_ENDPGM 0
+    SI_SPILL_A64_SAVE undef $agpr0_agpr1, %stack.0, $sgpr32, 0, implicit $exec :: (store (s64) into %stack.0, addrspace 5)
+    S_ENDPGM 0
+...
diff --git a/llvm/test/CodeGen/AMDGPU/vgpr-spill.mir b/llvm/test/CodeGen/AMDGPU/vgpr-spill.mir
index c2badc5720f149..edea344a66a3cd 100644
--- a/llvm/test/CodeGen/AMDGPU/vgpr-spill.mir
+++ b/llvm/test/CodeGen/AMDGPU/vgpr-spill.mir
@@ -153,3 +153,37 @@ body:             |
     ; CHECK-NEXT: BUFFER_STORE_DWORD_OFFSET killed $vgpr3, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr32, 12, 0, 0, implicit $exec, implicit killed $vgpr0_vgpr1_vgpr2_vgpr3 :: (store (s32) into %stack.0 + 12, addrspace 5)
     SI_SPILL_V128_SAVE killed $vgpr0_vgpr1_vgpr2_vgpr3, %stack.0, $sgpr32, 0, implicit $exec :: (store (s128) into %stack.0, addrspace 5)
 ...
+
+---
+name: spill_v32_undef
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, size: 4, alignment: 4 }
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  stackPtrOffsetReg: '$sgpr32'
+  frameOffsetReg: '$sgpr33'
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: spill_v32_undef
+    ; CHECK: S_NOP 0, implicit undef $vgpr0
+    SI_SPILL_V32_SAVE undef $vgpr0, %stack.0, $sgpr32, 0, implicit $exec :: (store (s32) into %stack.0, addrspace 5)
+    S_NOP 0, implicit undef $vgpr0
+...
+
+---
+name: spill_v64_undef
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, size: 8, alignment: 4 }
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  stackPtrOffsetReg: '$sgpr32'
+  frameOffsetReg: '$sgpr33'
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: spill_v64_undef
+    ; CHECK: S_NOP 0, implicit undef $vgpr0_vgpr1
+    SI_SPILL_V64_SAVE undef $vgpr0_vgpr1, %stack.0, $sgpr32, 0, implicit $exec :: (store (s64) into %stack.0, addrspace 5)
+    S_NOP 0, implicit undef $vgpr0_vgpr1
+...

>From 08b4cd552a6e1409a7181b9d4eddd8c0a5a540ff Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Thu, 12 Dec 2024 14:14:29 +0900
Subject: [PATCH 2/2] Move where undef sgpr spills are deleted

---
 llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp | 7 +++++++
 llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp    | 8 ++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp b/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp
index f868843318ff6c..d27c523425feb2 100644
--- a/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp
@@ -422,6 +422,13 @@ bool SILowerSGPRSpills::run(MachineFunction &MF) {
         if (!TII->isSGPRSpill(MI))
           continue;
 
+        if (MI.getOperand(0).isUndef()) {
+          if (Indexes)
+            Indexes->removeMachineInstrFromMaps(MI);
+          MI.eraseFromParent();
+          continue;
+        }
+
         int FI = TII->getNamedOperand(MI, AMDGPU::OpName::addr)->getIndex();
         assert(MFI.getStackID(FI) == TargetStackID::SGPRSpill);
 
diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
index 4f8c5c6756b3bb..8a23cf5e214967 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
@@ -1954,12 +1954,8 @@ bool SIRegisterInfo::spillSGPR(MachineBasicBlock::iterator MI, int Index,
                                RegScavenger *RS, SlotIndexes *Indexes,
                                LiveIntervals *LIS, bool OnlyToVGPR,
                                bool SpillToPhysVGPRLane) const {
-  if (MI->getOperand(0).isUndef()) {
-    if (Indexes)
-      Indexes->removeMachineInstrFromMaps(*MI);
-    MI->eraseFromParent();
-    return true;
-  }
+  assert(!MI->getOperand(0).isUndef() &&
+         "undef spill should have been deleted earlier");
 
   SGPRSpillBuilder SB(*this, *ST.getInstrInfo(), isWave32, MI, Index, RS);
 



More information about the llvm-branch-commits mailing list