[llvm] [AMDGPU] Save/Restore SCC bit across waterfall loop. (PR #68363)
Sirish Pande via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 12 08:18:59 PDT 2023
https://github.com/srpande updated https://github.com/llvm/llvm-project/pull/68363
>From 2d0d08517ce344b48952067bef85120494137a63 Mon Sep 17 00:00:00 2001
From: Sirish Pande <sirish.pande at amd.com>
Date: Tue, 3 Oct 2023 18:43:15 -0500
Subject: [PATCH 1/2] [AMDGPU] Use 32-bit SGPR to save/restore SCC.
SCC a bit in 32-bit STATUS register. Unless COPY's source or
destination is 64-bit, there is no need to use 64bit register.
Otherwise, it will just tie up a register unnecessarily,
which may cause register pressure in later passes.
---
llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp | 29 ++++++++----
llvm/test/CodeGen/AMDGPU/save_restore_scc.mir | 46 +++++++++++++++++++
2 files changed, 66 insertions(+), 9 deletions(-)
create mode 100644 llvm/test/CodeGen/AMDGPU/save_restore_scc.mir
diff --git a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
index 60cd9d4c3c35a27..645b6b3b374e0ec 100644
--- a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
@@ -1093,7 +1093,6 @@ void SIFixSGPRCopies::lowerVGPR2SGPRCopies(MachineFunction &MF) {
}
void SIFixSGPRCopies::fixSCCCopies(MachineFunction &MF) {
- bool IsWave32 = MF.getSubtarget<GCNSubtarget>().isWave32();
for (MachineFunction::iterator BI = MF.begin(), BE = MF.end(); BI != BE;
++BI) {
MachineBasicBlock *MBB = &*BI;
@@ -1106,13 +1105,18 @@ void SIFixSGPRCopies::fixSCCCopies(MachineFunction &MF) {
Register SrcReg = MI.getOperand(1).getReg();
Register DstReg = MI.getOperand(0).getReg();
if (SrcReg == AMDGPU::SCC) {
+ const TargetRegisterClass *DstRC =
+ TRI->getRegClassForOperandReg(*MRI, MI.getOperand(0));
+ unsigned DstRegSize = TRI->getRegSizeInBits(*DstRC);
+ assert((DstRegSize == 64 || DstRegSize == 32) &&
+ "Expected SCC dst to be 64 or 32 bits");
+ bool IsDst32Bit = DstRegSize == 32;
Register SCCCopy = MRI->createVirtualRegister(
- TRI->getRegClass(AMDGPU::SReg_1_XEXECRegClassID));
+ IsDst32Bit ? &AMDGPU::SReg_32RegClass : &AMDGPU::SReg_64RegClass);
+ unsigned Opcode =
+ IsDst32Bit ? AMDGPU::S_CSELECT_B32 : AMDGPU::S_CSELECT_B64;
I = BuildMI(*MI.getParent(), std::next(MachineBasicBlock::iterator(MI)),
- MI.getDebugLoc(),
- TII->get(IsWave32 ? AMDGPU::S_CSELECT_B32
- : AMDGPU::S_CSELECT_B64),
- SCCCopy)
+ MI.getDebugLoc(), TII->get(Opcode), SCCCopy)
.addImm(-1)
.addImm(0);
I = BuildMI(*MI.getParent(), std::next(I), I->getDebugLoc(),
@@ -1122,9 +1126,16 @@ void SIFixSGPRCopies::fixSCCCopies(MachineFunction &MF) {
continue;
}
if (DstReg == AMDGPU::SCC) {
- unsigned Opcode = IsWave32 ? AMDGPU::S_AND_B32 : AMDGPU::S_AND_B64;
- Register Exec = IsWave32 ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
- Register Tmp = MRI->createVirtualRegister(TRI->getBoolRC());
+ const TargetRegisterClass *SrcRC =
+ TRI->getRegClassForOperandReg(*MRI, MI.getOperand(1));
+ unsigned SrcRegSize = TRI->getRegSizeInBits(*SrcRC);
+ assert((SrcRegSize == 64 || SrcRegSize == 32) &&
+ "Expected SCC src to be 64 or 32 bits");
+ bool IsSrc32Bit = SrcRegSize == 32;
+ unsigned Opcode = IsSrc32Bit ? AMDGPU::S_AND_B32 : AMDGPU::S_AND_B64;
+ Register Exec = IsSrc32Bit ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
+ Register Tmp = MRI->createVirtualRegister(
+ IsSrc32Bit ? &AMDGPU::SReg_32RegClass : &AMDGPU::SReg_64RegClass);
I = BuildMI(*MI.getParent(), std::next(MachineBasicBlock::iterator(MI)),
MI.getDebugLoc(), TII->get(Opcode))
.addReg(Tmp, getDefRegState(true))
diff --git a/llvm/test/CodeGen/AMDGPU/save_restore_scc.mir b/llvm/test/CodeGen/AMDGPU/save_restore_scc.mir
new file mode 100644
index 000000000000000..7ee7cf05b95911d
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/save_restore_scc.mir
@@ -0,0 +1,46 @@
+# RUN: llc -march=amdgcn -mcpu=gfx906 -verify-machineinstrs -run-pass si-fix-sgpr-copies %s -o - | FileCheck %s -check-prefixes=GFX906
+# RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs -run-pass si-fix-sgpr-copies %s -o - | FileCheck %s -check-prefixes=GFX1030
+
+---
+
+# GFX1030-LABEL: name: waterfall_kills_scc_gfx1030
+# GFX1030: %1:sreg_32 = S_CSELECT_B32 -1, 0, implicit $scc
+# GFX1030: %2:sreg_32 = S_AND_B32 %0, $exec_lo, implicit-def $scc
+
+name: waterfall_kills_scc_gfx1030
+body: |
+ bb.0.entry:
+ successors: %bb.1(0x80000000)
+
+ %1:sreg_32 = COPY $scc
+
+ bb.1:
+ successors: %bb.1(0x80000000), %bb.2(0x40000000)
+
+ $exec = S_XOR_B64_term $exec, -1, implicit-def $scc
+ SI_WATERFALL_LOOP %bb.2, implicit $exec
+
+ bb.2:
+ $scc = COPY %1
+...
+
+# GFX906-LABEL: name: waterfall_kills_scc_gfx906
+# GFX906: %1:sreg_64 = S_CSELECT_B64 -1, 0, implicit $scc
+# GFX906: %2:sreg_64 = S_AND_B64 %0, $exec, implicit-def $scc
+---
+name: waterfall_kills_scc_gfx906
+body: |
+ bb.0.entry:
+ successors: %bb.1(0x80000000)
+
+ %1:sreg_64_xexec = COPY $scc
+
+ bb.1:
+ successors: %bb.1(0x80000000), %bb.2(0x40000000)
+
+ $exec = S_XOR_B64_term $exec, -1, implicit-def $scc
+ SI_WATERFALL_LOOP %bb.2, implicit $exec
+
+ bb.2:
+ $scc = COPY %1
+...
>From 5c5b8f0e266c6a37f3612828949c7abc130ead62 Mon Sep 17 00:00:00 2001
From: Sirish Pande <sirish.pande at amd.com>
Date: Thu, 28 Sep 2023 11:39:32 -0500
Subject: [PATCH 2/2] [AMDGPU] Save/Restore SCC bit across waterfall loop.
Waterfall loop is overwriting SCC bit of status register.
Make sure SCC bit is saved and restored. We need to save/restore
only in cases where SCC is live across waterfall loop.
---
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 20 ++++-
.../CodeGen/AMDGPU/waterfall_kills_scc.ll | 84 +++++++++++++++++++
2 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 llvm/test/CodeGen/AMDGPU/waterfall_kills_scc.ll
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 51397cbb791469d..b0b4de38b0e6103 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -6056,6 +6056,18 @@ loadMBUFScalarOperandsFromVGPR(const SIInstrInfo &TII, MachineInstr &MI,
unsigned MovExecOpc = ST.isWave32() ? AMDGPU::S_MOV_B32 : AMDGPU::S_MOV_B64;
const auto *BoolXExecRC = TRI->getRegClass(AMDGPU::SReg_1_XEXECRegClassID);
+ // Save SCC. Waterfall Loop may overwrite SCC.
+ Register SaveSCCReg;
+ bool SCCNotDead = (MBB.computeRegisterLiveness(TRI, AMDGPU::SCC, MI, 30) !=
+ MachineBasicBlock::LQR_Dead)
+ ? true
+ : false;
+ if (SCCNotDead) {
+ SaveSCCReg = MRI.createVirtualRegister(&AMDGPU::SReg_32RegClass);
+ BuildMI(MBB, Begin, DL, TII.get(AMDGPU::COPY), SaveSCCReg)
+ .addReg(AMDGPU::SCC);
+ }
+
Register SaveExec = MRI.createVirtualRegister(BoolXExecRC);
// Save the EXEC mask
@@ -6111,8 +6123,14 @@ loadMBUFScalarOperandsFromVGPR(const SIInstrInfo &TII, MachineInstr &MI,
emitLoadScalarOpsFromVGPRLoop(TII, MRI, MBB, *LoopBB, *BodyBB, DL, ScalarOps);
- // Restore the EXEC mask
MachineBasicBlock::iterator First = RemainderBB->begin();
+ // Restore SCC
+ if (SCCNotDead) {
+ BuildMI(*RemainderBB, First, DL, TII.get(AMDGPU::COPY), AMDGPU::SCC)
+ .addReg(SaveSCCReg);
+ }
+
+ // Restore the EXEC mask
BuildMI(*RemainderBB, First, DL, TII.get(MovExecOpc), Exec).addReg(SaveExec);
return BodyBB;
}
diff --git a/llvm/test/CodeGen/AMDGPU/waterfall_kills_scc.ll b/llvm/test/CodeGen/AMDGPU/waterfall_kills_scc.ll
new file mode 100644
index 000000000000000..4a12e8fd79a9a4d
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/waterfall_kills_scc.ll
@@ -0,0 +1,84 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 3
+; RUN: llc -march=amdgcn -mcpu=gfx906 -verify-machineinstrs < %s | FileCheck -check-prefix=GFX906 %s
+declare float @llvm.amdgcn.raw.buffer.load.f32(<4 x i32>, i32, i32, i32 immarg) #0
+declare void @llvm.amdgcn.raw.buffer.store.f32(float, <4 x i32>, i32, i32, i32 immarg) #1
+
+; Check that the compiler doesn't crash with a "undefined physical register" error;
+; bb.0 sets SCC bit in s_cmp_eq_u32 s0, 1
+; bb.1 overrides it
+; bb.2 uses the value from bb.0
+; Preserve SCC across bb.1 with s_cselect_b32 s5, -1, 0 -> s_and_b32 s0, s5, exec_lo
+; Otherwise, we will see the following error.
+;*** Bad machine code: Using an undefined physical register ***
+;- function: foo
+;- basic block: %bb.3 (0x53198c0)
+;- instruction: %33.sub1:sgpr_128 = S_CSELECT_B32 1072693248, 0, implicit $scc
+;- operand 3: implicit $scc
+
+
+define amdgpu_kernel void @foo(i1 %cmp1) {
+; GFX906-LABEL: foo:
+; GFX906: ; %bb.0: ; %entry
+; GFX906-NEXT: s_mov_b32 s8, SCRATCH_RSRC_DWORD0
+; GFX906-NEXT: s_mov_b32 s9, SCRATCH_RSRC_DWORD1
+; GFX906-NEXT: s_mov_b32 s10, -1
+; GFX906-NEXT: s_mov_b32 s11, 0xe00000
+; GFX906-NEXT: s_add_u32 s8, s8, s3
+; GFX906-NEXT: s_addc_u32 s9, s9, 0
+; GFX906-NEXT: buffer_load_dword v3, off, s[8:11], 0
+; GFX906-NEXT: buffer_load_dword v4, off, s[8:11], 0 offset:4
+; GFX906-NEXT: buffer_load_dword v5, off, s[8:11], 0 offset:8
+; GFX906-NEXT: buffer_load_dword v6, off, s[8:11], 0 offset:12
+; GFX906-NEXT: s_load_dword s4, s[0:1], 0x24
+; GFX906-NEXT: s_load_dwordx2 s[2:3], s[0:1], 0x1c
+; GFX906-NEXT: s_waitcnt lgkmcnt(0)
+; GFX906-NEXT: s_bitcmp1_b32 s4, 0
+; GFX906-NEXT: s_mul_i32 s0, s2, s3
+; GFX906-NEXT: v_mul_u32_u24_e32 v1, s3, v1
+; GFX906-NEXT: v_mad_u32_u24 v0, s0, v0, v1
+; GFX906-NEXT: v_add_lshl_u32 v2, v0, v2, 4
+; GFX906-NEXT: v_mov_b32_e32 v0, 0
+; GFX906-NEXT: s_mov_b32 s4, 0
+; GFX906-NEXT: v_mov_b32_e32 v1, v0
+; GFX906-NEXT: s_cselect_b32 s5, -1, 0
+; GFX906-NEXT: s_mov_b64 s[2:3], exec
+; GFX906-NEXT: ds_write_b64 v2, v[0:1]
+; GFX906-NEXT: .LBB0_1: ; =>This Inner Loop Header: Depth=1
+; GFX906-NEXT: s_waitcnt vmcnt(3)
+; GFX906-NEXT: v_readfirstlane_b32 s0, v3
+; GFX906-NEXT: s_waitcnt vmcnt(2)
+; GFX906-NEXT: v_readfirstlane_b32 s1, v4
+; GFX906-NEXT: v_cmp_eq_u64_e32 vcc, s[0:1], v[3:4]
+; GFX906-NEXT: s_waitcnt vmcnt(1)
+; GFX906-NEXT: v_readfirstlane_b32 s0, v5
+; GFX906-NEXT: s_waitcnt vmcnt(0)
+; GFX906-NEXT: v_readfirstlane_b32 s1, v6
+; GFX906-NEXT: v_cmp_eq_u64_e64 s[0:1], s[0:1], v[5:6]
+; GFX906-NEXT: s_and_b64 s[0:1], vcc, s[0:1]
+; GFX906-NEXT: s_and_saveexec_b64 s[0:1], s[0:1]
+; GFX906-NEXT: ; implicit-def: $vgpr3_vgpr4_vgpr5_vgpr6
+; GFX906-NEXT: s_xor_b64 exec, exec, s[0:1]
+; GFX906-NEXT: s_cbranch_execnz .LBB0_1
+; GFX906-NEXT: ; %bb.2:
+; GFX906-NEXT: s_and_b32 s0, s5, exec_lo
+; GFX906-NEXT: s_mov_b64 exec, s[2:3]
+; GFX906-NEXT: s_cselect_b32 s5, 0x3ff00000, 0
+; GFX906-NEXT: v_cvt_f32_f64_e32 v0, s[4:5]
+; GFX906-NEXT: s_mov_b32 s5, s4
+; GFX906-NEXT: s_mov_b32 s6, s4
+; GFX906-NEXT: s_mov_b32 s7, s4
+; GFX906-NEXT: buffer_store_dword v0, off, s[4:7], 0
+; GFX906-NEXT: s_endpgm
+entry:
+ %wbr = alloca <4 x i32>, align 16, addrspace(5)
+ store ptr null, ptr addrspace(5) %wbr, align 16
+ %wbr_1 = load <4 x i32>, ptr addrspace(5) null, align 16
+ %call1 = tail call float @llvm.amdgcn.raw.buffer.load.f32(<4 x i32> %wbr_1, i32 0, i32 0, i32 0)
+ %0 = fpext float %call1 to double
+ %sel1 = select i1 %cmp1, double 1.000000e+00, double 0.000000e+00
+ %sel2 = select i1 %cmp1, double %0, double 0.000000e+00
+ %mul = fmul double %sel2, 0.000000e+00
+ %fptruncate = fptrunc double %sel1 to float
+ tail call void @llvm.amdgcn.raw.buffer.store.f32(float %fptruncate, <4 x i32> zeroinitializer, i32 0, i32 0, i32 0)
+ ret void
+}
More information about the llvm-commits
mailing list