[llvm] [AMDGPU] Account for existing SDWA selections (PR #123221)
Frederik Harwath via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 30 00:38:29 PST 2025
================
@@ -0,0 +1,124 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=amdgcn -mcpu=gfx1030 -run-pass=si-peephole-sdwa -o - %s | FileCheck -check-prefix=NOHAZARD %s
+
+---
+name: sdwa_opsel_hazard
+body: |
+ ; NOHAZARD-LABEL: name: sdwa_opsel_hazard
+ ; NOHAZARD: bb.0:
+ ; NOHAZARD-NEXT: successors: %bb.7(0x40000000), %bb.8(0x40000000)
+ ; NOHAZARD-NEXT: liveins: $vgpr0, $sgpr4_sgpr5, $sgpr6
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: [[DEF:%[0-9]+]]:sreg_32 = IMPLICIT_DEF
+ ; NOHAZARD-NEXT: [[DEF1:%[0-9]+]]:sreg_64_xexec_xnull = IMPLICIT_DEF
+ ; NOHAZARD-NEXT: [[DEF2:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+ ; NOHAZARD-NEXT: [[GLOBAL_LOAD_DWORD_SADDR:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR killed [[DEF1]], [[DEF2]], 0, 0, implicit $exec
+ ; NOHAZARD-NEXT: [[SI_IF:%[0-9]+]]:sreg_32 = SI_IF undef [[DEF]], %bb.8, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; NOHAZARD-NEXT: S_BRANCH %bb.7
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.1:
+ ; NOHAZARD-NEXT: successors: %bb.2(0x80000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 255, implicit $exec
+ ; NOHAZARD-NEXT: [[V_AND_B32_sdwa:%[0-9]+]]:vgpr_32 = V_AND_B32_sdwa 0, undef [[GLOBAL_LOAD_DWORD_SADDR]], 0, [[V_MOV_B32_e32_]], 0, 6, 0, 5, 6, implicit $exec
+ ; NOHAZARD-NEXT: [[V_MOV_B32_e32_1:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 2, implicit $exec
+ ; NOHAZARD-NEXT: [[V_LSHLREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHLREV_B32_sdwa 0, [[V_MOV_B32_e32_1]], 0, undef [[GLOBAL_LOAD_DWORD_SADDR]], 0, 6, 0, 6, 2, implicit $exec
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.2:
+ ; NOHAZARD-NEXT: successors: %bb.3(0x40000000), %bb.4(0x40000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: [[SI_IF1:%[0-9]+]]:sreg_32 = SI_IF killed undef %9, %bb.4, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; NOHAZARD-NEXT: S_BRANCH %bb.3
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.3:
+ ; NOHAZARD-NEXT: successors: %bb.4(0x80000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.4:
+ ; NOHAZARD-NEXT: successors: %bb.5(0x40000000), %bb.6(0x40000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: [[SI_IF2:%[0-9]+]]:sreg_32 = SI_IF killed undef [[SI_IF1]], %bb.6, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; NOHAZARD-NEXT: S_BRANCH %bb.5
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.5:
+ ; NOHAZARD-NEXT: successors: %bb.6(0x80000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.6:
+ ; NOHAZARD-NEXT: successors: %bb.9(0x40000000), %bb.10(0x40000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: [[SI_IF3:%[0-9]+]]:sreg_32 = SI_IF undef [[DEF]], %bb.10, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; NOHAZARD-NEXT: S_BRANCH %bb.9
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.7:
+ ; NOHAZARD-NEXT: successors: %bb.8(0x80000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.8:
+ ; NOHAZARD-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: [[V_LSHRREV_B32_e64_:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_e64 16, undef [[GLOBAL_LOAD_DWORD_SADDR]], implicit $exec
+ ; NOHAZARD-NEXT: [[SI_IF4:%[0-9]+]]:sreg_32 = SI_IF killed undef [[SI_IF]], %bb.2, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; NOHAZARD-NEXT: S_BRANCH %bb.1
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.9:
+ ; NOHAZARD-NEXT: successors: %bb.10(0x80000000)
+ ; NOHAZARD-NEXT: {{ $}}
+ ; NOHAZARD-NEXT: bb.10:
+ ; NOHAZARD-NEXT: S_ENDPGM 0
+ bb.0:
+ successors: %bb.7(0x40000000), %bb.8(0x40000000)
+ liveins: $vgpr0, $sgpr4_sgpr5, $sgpr6
+
+ %0:sreg_32 = IMPLICIT_DEF
+ %1:sreg_64_xexec_xnull = IMPLICIT_DEF
+ %2:vgpr_32 = IMPLICIT_DEF
+ %3:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR killed %1, %2, 0, 0, implicit $exec
+ %4:sreg_32 = SI_IF undef %0, %bb.8, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.7
+
+ bb.1:
+ successors: %bb.2(0x80000000)
+
+ %5:vgpr_32 = V_AND_B32_e64 undef %6, 255, implicit $exec
+ %7:vgpr_32 = V_LSHLREV_B32_e64 2, killed undef %5, implicit $exec
+
+ bb.2:
+ successors: %bb.3(0x40000000), %bb.4(0x40000000)
+
+ %8:sreg_32 = SI_IF killed undef %9, %bb.4, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.3
+
+ bb.3:
+ successors: %bb.4(0x80000000)
+
+ bb.4:
+ successors: %bb.5(0x40000000), %bb.6(0x40000000)
+
+ %10:sreg_32 = SI_IF killed undef %8, %bb.6, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.5
+
+ bb.5:
+ successors: %bb.6(0x80000000)
+
+ bb.6:
+ successors: %bb.9(0x40000000), %bb.10(0x40000000)
+
+ %11:sreg_32 = SI_IF undef %0, %bb.10, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.9
+
+ bb.7:
+ successors: %bb.8(0x80000000)
+
+ bb.8:
+ successors: %bb.1(0x40000000), %bb.2(0x40000000)
+
+ %6:vgpr_32 = V_LSHRREV_B32_e64 16, undef %3, implicit $exec
+ %9:sreg_32 = SI_IF killed undef %4, %bb.2, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.1
+
+ bb.9:
+ successors: %bb.10(0x80000000)
+
+ bb.10:
+ S_ENDPGM 0
+
+...
+
----------------
frederik-h wrote:
> There's a lot going on in this MIR, do we really need control flow? I'd also like an end to end IR test for any miscompiles
I think the control flow is necessary to reproduce the issue. The `SIPeepholeSDWA::run(MachineFunction &MF)` function changes the instructions within a single basic block until a fixed point is reached, but the basic blocks are only visited once, i.e. the loop structure looks like this:
```cpp
// Find all SDWA operands in MF.
bool Ret = false;
for (MachineBasicBlock &MBB : MF) {
bool Changed = false;
do {
matchSDWAOperands(MBB);
[...]
} while (Changed);
}
```
Here is minimized reproducer for the incorrect sdwa selections which was derived from original program that produced a wrong output because of this issue:
```cpp
const int n = 4;
__device__ void u(int *ap, uint8_t (&at)[n]) {
for (int i = n - 1; i; --i) {
if (at[i - 1])
ap[at[i]] = i;
}
}
struct al {
static __global__ void ar(uint8_t *aa) {
uint8_t at[n];
__shared__ int *ap;
// This used to be a loop but unrolling it
// preserves the problematic sdwa instructions:
// for (int i = 0; i < n; i++)
// at[i] = aa[threadIdx.x + i];
at[0] = aa[threadIdx.x + 0];
at[1] = aa[threadIdx.x + 1];
at[2] = aa[threadIdx.x + 2];
at[3] = aa[threadIdx.x + 3];
// Some loop and conditional statement
// with a body that accesses ap seems to be
// necessary to reproduce the issue
// since the sdwa instructions disappear otherwise.
// For instance, this would work as well:
// for (int i = 0; i < 2; i++)
// if (!aa)
// ap = (int*)nullptr;
for (int i = 0; i < n; i++) {
if (threadIdx.x == i)
ap[i] = 0;
}
// If the call is inlined, sdwa instructions are still used, but the sdwa selection combination
// has no effect.
u(ap, at);
}
};
```
The difference in the instructions between the baseline and the version produced by the code from this PR is:
```diff
< v_lshlrev_b32_sdwa v6, v5, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:WORD_1
---
> v_lshlrev_b32_sdwa v6, v5, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_2
```
https://github.com/llvm/llvm-project/pull/123221
More information about the llvm-commits
mailing list