[llvm] [AMDGPU] Fix checkVALUHazardsHelper distance accounting on branchy CFG (PR #203770)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 14 17:42:49 PDT 2026
https://github.com/macurtis-amd updated https://github.com/llvm/llvm-project/pull/203770
>From 06a3b0cb974dfd79c25e55df2964e6d3ed74c209 Mon Sep 17 00:00:00 2001
From: Matthew Curtis <macurtis at amd.com>
Date: Sun, 14 Jun 2026 19:05:32 -0500
Subject: [PATCH] [AMDGPU] Fix checkVALUHazardsHelper distance accounting on
branchy CFG
#197267's checkVALUHazardsHelper rewrite walked predecessors with an
always-false predicate and accumulated wait-state distance in a single
counter shared across recursively visited blocks. On branchy control
flow that counter sums sibling-path distances, so window - distance goes
negative and the protective S_NOP is dropped.
Restore independent per-window getWaitStatesSince scans, which return
the minimum distance to a matching producer across predecessor paths.
Split the pre-gfx940 and gfx940-family paths for readability while
preserving #197267's sgpr-soffset window rules.
Add a cross-block MIR regression test; it fails without this fix on
gfx942/gfx950.
Co-authored-by: Larry Meadows <Lawrence.Meadows at amd.com>
---
.../lib/Target/AMDGPU/GCNHazardRecognizer.cpp | 98 +++++++------
llvm/lib/Target/AMDGPU/GCNHazardRecognizer.h | 2 +
...tore-dwordx4-vpk-mul-war-hazard-gfx942.mir | 130 ++++++++++++++++++
3 files changed, 187 insertions(+), 43 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
index 25a7ca7d957b0..829013ca01596 100644
--- a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
@@ -932,65 +932,77 @@ int GCNHazardRecognizer::createsVALUHazard(const MachineInstr &MI) const {
return -1;
}
-int GCNHazardRecognizer::checkVALUHazardsHelper(
- const MachineOperand &Def, const MachineRegisterInfo &MRI) const {
- // Helper to check for the hazard where VMEM instructions that store more
- // than 8 bytes can have their store data overwritten by the next
- // instruction. On gfx940-family the window depends on the producer's
+int GCNHazardRecognizer::checkPreGFX940VALUHazardsHelper(Register Reg) const {
+ // Pre-gfx940 wide stores need a single wait-state bubble before a VALU that
+ // overwrites store data. createsVALUHazard already excludes MUBUF/MTBUF
+ // stores with an SGPR SOFFSET.
+ const SIRegisterInfo *TRI = ST.getRegisterInfo();
+
+ auto IsHazard = [this, TRI, Reg](const MachineInstr &MI) {
+ int DataIdx = createsVALUHazard(MI);
+ return DataIdx >= 0 &&
+ TRI->regsOverlap(MI.getOperand(DataIdx).getReg(), Reg);
+ };
+
+ return std::max(0, 1 - getWaitStatesSince(IsHazard, 1));
+}
+
+int GCNHazardRecognizer::checkGFX940FamilyVALUHazardsHelper(
+ Register Reg) const {
+ // On gfx940-family the required wait-state window depends on the producer's
// SOFFSET shape:
// - MUBUF/MTBUF wide store with sgpr SOFFSET: 1 wait state.
// - MUBUF/MTBUF wide store with literal/absent SOFFSET, and FLAT wide
// store: 2 wait states.
- // Pre-gfx940 keeps a single 1-wait-state window. The 1-cycle sgpr-SOFFSET
- // window was measured on gfx950 (MI350X); the same gate is applied to the
- // rest of the gfx940 family to match the existing rule's granularity.
+ // The 1-cycle sgpr-SOFFSET window was measured on gfx950 (MI350X); the same
+ // gate is applied to the rest of the gfx940 family to match the existing
+ // rule's granularity.
const SIRegisterInfo *TRI = ST.getRegisterInfo();
const SIInstrInfo *TII = ST.getInstrInfo();
int WaitStatesNeeded = 0;
- if (!TRI->isVectorRegister(MRI, Def.getReg()))
- return WaitStatesNeeded;
- const Register Reg = Def.getReg();
- const int MaxWaitStates = ST.hasGFX940Insts() ? 2 : 1;
+ // Scan each wait-state window separately and take the max padding needed.
+ // getWaitStatesSince supplies the minimum distance to a producer over paths.
+ for (int Window = 1; Window <= 2; ++Window) {
+ auto IsHazard = [this, TII, TRI, Reg, Window](const MachineInstr &MI) {
+ int DataIdx = createsVALUHazard(MI);
+ if (DataIdx < 0 ||
+ !TRI->regsOverlap(MI.getOperand(DataIdx).getReg(), Reg))
+ return false;
+
+ // Window 1 matches every hazard producer. Window 2 excludes BUF stores
+ // with an SGPR SOFFSET, which only require a single wait state.
+ if (Window == 1 || !TII->isBUF(MI))
+ return true;
- // Per-producer required wait-state window. On pre-gfx940 every producer
- // uses 1; on gfx940-family MUBUF/MTBUF stores with an SGPR SOFFSET use 1
- // and everything else (literal/absent SOFFSET, FLAT) uses 2.
- auto WindowFor = [this, TII](const MachineInstr &MI) -> int {
- if (!ST.hasGFX940Insts())
- return 1;
- if (TII->isBUF(MI)) {
const MachineOperand *SOffset =
TII->getNamedOperand(MI, AMDGPU::OpName::soffset);
- if (SOffset && SOffset->isReg())
- return 1;
- }
- return 2;
- };
-
- // For each hazard producer reached, accumulate the wait states still
- // needed using that producer's own window. The predicate always returns
- // false so the walk runs to MaxWaitStates.
- int Distance = 0;
- auto Counter = [&](const MachineInstr &MI) {
- int DataIdx = createsVALUHazard(MI);
- if (DataIdx >= 0 &&
- TRI->regsOverlap(MI.getOperand(DataIdx).getReg(), Reg)) {
- int Need = WindowFor(MI) - Distance;
- WaitStatesNeeded = std::max(WaitStatesNeeded, Need);
- }
- // Mirror getWaitStatesSince's accounting, which does not count inline asm
- // towards the wait-state distance.
- if (!MI.isInlineAsm())
- Distance += SIInstrInfo::getNumWaitStates(MI);
- return false;
- };
- getWaitStatesSince(Counter, MaxWaitStates);
+ return !SOffset || !SOffset->isReg();
+ };
+ WaitStatesNeeded = std::max(WaitStatesNeeded,
+ Window - getWaitStatesSince(IsHazard, Window));
+ }
return WaitStatesNeeded;
}
+int GCNHazardRecognizer::checkVALUHazardsHelper(
+ const MachineOperand &Def, const MachineRegisterInfo &MRI) const {
+ // Helper to check for the hazard where VMEM instructions that store more
+ // than 8 bytes can have their store data overwritten by the next
+ // instruction.
+ const SIRegisterInfo *TRI = ST.getRegisterInfo();
+
+ if (!TRI->isVectorRegister(MRI, Def.getReg()))
+ return 0;
+
+ if (!ST.hasGFX940Insts())
+ return checkPreGFX940VALUHazardsHelper(Def.getReg());
+
+ return checkGFX940FamilyVALUHazardsHelper(Def.getReg());
+}
+
/// Dest sel forwarding issue occurs if additional logic is needed to swizzle /
/// pack the computed value into correct bit position of the dest register. This
/// occurs if we have SDWA with dst_sel != DWORD or if we have op_sel with
diff --git a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.h b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.h
index 920939a81db7f..145223b83f1c9 100644
--- a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.h
+++ b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.h
@@ -99,6 +99,8 @@ class GCNHazardRecognizer final : public ScheduleHazardRecognizer {
int checkVALUHazards(MachineInstr *VALU) const;
int checkVALUHazardsHelper(const MachineOperand &Def,
const MachineRegisterInfo &MRI) const;
+ int checkPreGFX940VALUHazardsHelper(Register Reg) const;
+ int checkGFX940FamilyVALUHazardsHelper(Register Reg) const;
int checkRWLaneHazards(MachineInstr *RWLane) const;
int checkRFEHazards(MachineInstr *RFE) const;
int checkInlineAsmHazards(MachineInstr *IA) const;
diff --git a/llvm/test/CodeGen/AMDGPU/buffer-store-dwordx4-vpk-mul-war-hazard-gfx942.mir b/llvm/test/CodeGen/AMDGPU/buffer-store-dwordx4-vpk-mul-war-hazard-gfx942.mir
index db8b47f5cab5f..a05296ba81f23 100644
--- a/llvm/test/CodeGen/AMDGPU/buffer-store-dwordx4-vpk-mul-war-hazard-gfx942.mir
+++ b/llvm/test/CodeGen/AMDGPU/buffer-store-dwordx4-vpk-mul-war-hazard-gfx942.mir
@@ -120,3 +120,133 @@ body: |
$vgpr10_vgpr11 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
S_ENDPGM 0
...
+
+# Cross-block regression test. The wide store and the clobbering VALU are in
+# different basic blocks: the store is on one arm of a diamond (%bb.2) and the
+# V_PK_MUL that overwrites its source vgprs is in the join block (%bb.3). The
+# hazard must be protected because at runtime the %bb.2 -> %bb.3 edge places the
+# store within the wait-state window of the V_PK_MUL.
+#
+# This is the shape that regressed: the wait-state distance must be the *minimum*
+# over predecessor paths. An implementation that accumulates distance into a
+# single counter shared across the recursively-visited predecessor arms
+# (%bb.1 here, visited first) over-counts, computes a non-positive remaining
+# window, and drops the required S_NOP -- leaving the gfx940-family
+# BUFFER_STORE source-vgpr WAR hazard unprotected.
+
+---
+name: buffer_store_dwordx4_literal_soffset_then_vpk_mul_across_branch
+tracksRegLiveness: true
+body: |
+ ; GFX950-LABEL: name: buffer_store_dwordx4_literal_soffset_then_vpk_mul_across_branch
+ ; GFX950: bb.0:
+ ; GFX950-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ ; GFX950-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $vgpr8_vgpr9, $sgpr0_sgpr1, $sgpr8_sgpr9_sgpr10_sgpr11, $scc
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: S_CBRANCH_SCC1 %bb.2, implicit $scc
+ ; GFX950-NEXT: S_BRANCH %bb.1
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: bb.1:
+ ; GFX950-NEXT: successors: %bb.3(0x80000000)
+ ; GFX950-NEXT: liveins: $vgpr8_vgpr9, $sgpr0_sgpr1
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: $vgpr10_vgpr11 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX950-NEXT: $vgpr12_vgpr13 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX950-NEXT: S_BRANCH %bb.3
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: bb.2:
+ ; GFX950-NEXT: successors: %bb.3(0x80000000)
+ ; GFX950-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: BUFFER_STORE_DWORDX4_OFFEN_exact $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11, 0, 0, 0, 0, implicit $exec
+ ; GFX950-NEXT: S_BRANCH %bb.3
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: bb.3:
+ ; GFX950-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr8_vgpr9, $sgpr0_sgpr1
+ ; GFX950-NEXT: {{ $}}
+ ; GFX950-NEXT: S_NOP 0
+ ; GFX950-NEXT: $vgpr0_vgpr1 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX950-NEXT: S_ENDPGM 0
+ ;
+ ; GFX942-LABEL: name: buffer_store_dwordx4_literal_soffset_then_vpk_mul_across_branch
+ ; GFX942: bb.0:
+ ; GFX942-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ ; GFX942-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $vgpr8_vgpr9, $sgpr0_sgpr1, $sgpr8_sgpr9_sgpr10_sgpr11, $scc
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: S_CBRANCH_SCC1 %bb.2, implicit $scc
+ ; GFX942-NEXT: S_BRANCH %bb.1
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: bb.1:
+ ; GFX942-NEXT: successors: %bb.3(0x80000000)
+ ; GFX942-NEXT: liveins: $vgpr8_vgpr9, $sgpr0_sgpr1
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: $vgpr10_vgpr11 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX942-NEXT: $vgpr12_vgpr13 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX942-NEXT: S_BRANCH %bb.3
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: bb.2:
+ ; GFX942-NEXT: successors: %bb.3(0x80000000)
+ ; GFX942-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: BUFFER_STORE_DWORDX4_OFFEN_exact $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11, 0, 0, 0, 0, implicit $exec
+ ; GFX942-NEXT: S_BRANCH %bb.3
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: bb.3:
+ ; GFX942-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr8_vgpr9, $sgpr0_sgpr1
+ ; GFX942-NEXT: {{ $}}
+ ; GFX942-NEXT: S_NOP 0
+ ; GFX942-NEXT: $vgpr0_vgpr1 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX942-NEXT: S_ENDPGM 0
+ ;
+ ; GFX900-LABEL: name: buffer_store_dwordx4_literal_soffset_then_vpk_mul_across_branch
+ ; GFX900: bb.0:
+ ; GFX900-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ ; GFX900-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $vgpr8_vgpr9, $sgpr0_sgpr1, $sgpr8_sgpr9_sgpr10_sgpr11, $scc
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: S_CBRANCH_SCC1 %bb.2, implicit $scc
+ ; GFX900-NEXT: S_BRANCH %bb.1
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: bb.1:
+ ; GFX900-NEXT: successors: %bb.3(0x80000000)
+ ; GFX900-NEXT: liveins: $vgpr8_vgpr9, $sgpr0_sgpr1
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: $vgpr10_vgpr11 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX900-NEXT: $vgpr12_vgpr13 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX900-NEXT: S_BRANCH %bb.3
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: bb.2:
+ ; GFX900-NEXT: successors: %bb.3(0x80000000)
+ ; GFX900-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: BUFFER_STORE_DWORDX4_OFFEN_exact $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11, 0, 0, 0, 0, implicit $exec
+ ; GFX900-NEXT: S_BRANCH %bb.3
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: bb.3:
+ ; GFX900-NEXT: liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr8_vgpr9, $sgpr0_sgpr1
+ ; GFX900-NEXT: {{ $}}
+ ; GFX900-NEXT: $vgpr0_vgpr1 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ ; GFX900-NEXT: S_ENDPGM 0
+ bb.0:
+ successors: %bb.1, %bb.2
+ liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $vgpr8_vgpr9, $sgpr0_sgpr1, $sgpr8_sgpr9_sgpr10_sgpr11, $scc
+ S_CBRANCH_SCC1 %bb.2, implicit $scc
+ S_BRANCH %bb.1
+
+ bb.1:
+ successors: %bb.3
+ liveins: $vgpr8_vgpr9, $sgpr0_sgpr1
+ $vgpr10_vgpr11 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ $vgpr12_vgpr13 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ S_BRANCH %bb.3
+
+ bb.2:
+ successors: %bb.3
+ liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11
+ BUFFER_STORE_DWORDX4_OFFEN_exact $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr4, $sgpr8_sgpr9_sgpr10_sgpr11, 0, 0, 0, 0, implicit $exec
+ S_BRANCH %bb.3
+
+ bb.3:
+ liveins: $vgpr0_vgpr1_vgpr2_vgpr3, $vgpr8_vgpr9, $sgpr0_sgpr1
+ $vgpr0_vgpr1 = nofpexcept V_PK_MUL_F32 0, $sgpr0_sgpr1, 8, $vgpr8_vgpr9, 0, 0, 0, 0, 0, implicit $mode, implicit $exec
+ S_ENDPGM 0
+...
More information about the llvm-commits
mailing list