https://github.com/arsenm created https://github.com/llvm/llvm-project/pull/119684
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 Move where undef sgpr spills are deleted >From ea9cd242330d6938eb3097192cc6e723b43c01fa Mon Sep 17 00:00:00 2001 From: Matt Arsenault <matthew.arsena...@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 8a05688cca3e32da6e2fe32e847818659881275b Mon Sep 17 00:00:00 2001 From: Matt Arsenault <matthew.arsena...@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); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
