[llvm] [AMDGPU] Fix checkVALUHazardsHelper distance accounting on branchy CFG (PR #203770)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 14 07:05:50 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: macurtis-amd
<details>
<summary>Changes</summary>
The `checkVALUHazardsHelper` rewrite accumulated wait-state distance in a single counter shared across recursively-visited predecessor blocks, so on branchy control flow it over-counts and drops the protective `S_NOP`.
This fix restores per-window getWaitStatesSince min-over-paths accounting while preserving #<!-- -->197267's sgpr-soffset windows.
---
Full diff: https://github.com/llvm/llvm-project/pull/203770.diff
2 Files Affected:
- (modified) llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp (+31-26)
- (modified) llvm/test/CodeGen/AMDGPU/buffer-store-dwordx4-vpk-mul-war-hazard-gfx942.mir (+130)
``````````diff
diff --git a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
index 25a7ca7d957b0..41e6ab8372e51 100644
--- a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
@@ -936,8 +936,8 @@ 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
- // SOFFSET shape:
+ // instruction. 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.
@@ -952,12 +952,13 @@ int GCNHazardRecognizer::checkVALUHazardsHelper(
return WaitStatesNeeded;
const Register Reg = Def.getReg();
- const int MaxWaitStates = ST.hasGFX940Insts() ? 2 : 1;
-
- // 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 {
+ // Wait-state window required by a hazard producer that overwrites Reg.
+ // Returns 0 when MI is not a hazard producer for Reg.
+ auto WindowFor = [this, TII, TRI, Reg](const MachineInstr &MI) -> int {
+ int DataIdx = createsVALUHazard(MI);
+ if (DataIdx < 0 ||
+ !TRI->regsOverlap(MI.getOperand(DataIdx).getReg(), Reg))
+ return 0;
if (!ST.hasGFX940Insts())
return 1;
if (TII->isBUF(MI)) {
@@ -969,24 +970,28 @@ int GCNHazardRecognizer::checkVALUHazardsHelper(
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);
+ const int MaxWaitStates = ST.hasGFX940Insts() ? 2 : 1;
+
+ // Evaluate each window size independently, letting getWaitStatesSince do the
+ // walk with its own hazard predicate. getWaitStatesSince returns the
+ // *minimum* wait-state distance to a matching producer across all
+ // predecessor paths, which is exactly the property required for correctness
+ // under control flow. A producer needing N wait states matches every window
+ // 1..N, so taking the max over windows applies its full requirement.
+ //
+ // The previous implementation instead passed an always-false predicate and
+ // accumulated the distance as a side effect through a single counter that
+ // was shared across all recursively-visited predecessor blocks. On branchy
+ // code that counter summed distances from sibling paths, so
+ // `window - distance` went negative and the required nop was dropped,
+ // leaving the gfx940 BUFFER_STORE WAR hazard unprotected.
+ for (int Window = 1; Window <= MaxWaitStates; ++Window) {
+ auto IsHazardFn = [&WindowFor, Window](const MachineInstr &MI) {
+ return WindowFor(MI) >= Window;
+ };
+ WaitStatesNeeded = std::max(WaitStatesNeeded,
+ Window - getWaitStatesSince(IsHazardFn, Window));
+ }
return WaitStatesNeeded;
}
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
+...
``````````
</details>
https://github.com/llvm/llvm-project/pull/203770
More information about the llvm-commits
mailing list