Skip to content

Commit e66f1cb

Browse files
jlebarclaude
andcommitted
m150: s_barrier_init/signal_var M0 mask discarded immediately
Found by amdgcn.s.barrier audit. SIISelLowering.cpp:12450-12459 (lowering of amdgcn.s.barrier.init and amdgcn.s.barrier.signal.var): M0Val = AND(CntOp, 0x3F); // (1) -- sets M0Val to masked CntOp M0Val = SHL(CntOp, 16); // (2) -- OVERWRITES with UNMASKED CntOp Line (1) is dead code; the intended (CntOp & 0x3F) << 16 expression is never produced. If member-count has bits >=6 set (e.g. 0x40, 0x80, 0x100), those bits land in M0[27:22], corrupting unrelated fields when the HW decodes M0. Fix: M0Val = AND(CntOp, 0x3F); M0Val = SHL(M0Val, 16); // shift the MASKED value M0Val = OR(M0Val, BarID); // compose with barrier ID Adjacent (not filed): SOPInstructions.td:507-573 -- S_BARRIER and split-barrier pseudos lack mayLoad/mayStore, combined with IntrNoMem on the intrinsics, MachineScheduler can reorder global/flat/LDS memops across the barrier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e630ef0 commit e66f1cb

3 files changed

Lines changed: 148 additions & 0 deletions

File tree

‎amdgpu/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ Tested toolchains as of 2026-05-19:
444444
| [m141-iscanonicalized-bitcast-loses-fp-type](known-miscompiles/m141-iscanonicalized-bitcast-loses-fp-type/NOTES.md) | ❌ | ❌ | ❌ | `SITargetLowering::isCanonicalized` recurses through `ISD::BITCAST` (`SIISelLowering.cpp:15649-15653`) WITHOUT consulting source/dest FP semantics. In-source TODO acknowledges the bug. v2bf16 (8-bit exp) and v2f16 (5-bit exp) have different denormal ranges, so the same 16-bit pattern can be normal in one type and denormal in the other -- and a NaN-payload bit-pattern in one type may not be canonical-NaN in the other. The function gates `is_canonicalized_1/2` PatFrags (`AMDGPUInstructions.td:189,207`; `SIInstrInfo.td:1017,1025`) which decide whether `V_PACK_B32_F16` / `min/max` selection may omit the explicit canonicalise. Effect: O2 drops the canonicalize and lets a denormal/sNaN through where O0 emits `v_pk_max_f16 v, v` that FTZs/quiets. Sibling of m118 (same function, different arm), m115/m124, m133. |
445445
| [m142-image-d16-bf16-skipped](known-miscompiles/m142-image-d16-bf16-skipped/NOTES.md) | ❌ | ❌ | ❌ | `SITargetLowering::lowerImage` D16 detection at `SIISelLowering.cpp:10190` (store) / `10203` (load) / `11926` (handleD16VData) / `12056/12084/12120/12170` (TBUFFER) uses `getScalarType() == MVT::f16` only. bf16 is a distinct MVT, so `<N x bfloat>` image data/result silently skips `handleD16VData`, selects the non-D16 MIMG opcode, computes wrong NumVDataDwords, and bypasses the `HasD16` guard. GISel correctly handles bf16 via `getScalarType() == S16` (`AMDGPULegalizerInfo.cpp:7182`) -- SDAG-only miscompile. Same IR, same bits, different image opcode depending on `-global-isel`. No upstream lit test covers bf16 image. |
446446
| [m143-strict-fp-round-f64-bf16-drops-chain](known-miscompiles/m143-strict-fp-round-f64-bf16-drops-chain/NOTES.md) | ❌ | ❌ | ❌ | `SITargetLowering::lowerFP_ROUND` (`SIISelLowering.cpp:8604-8613`, f64->bf16 path) lacks the strictfp-bail guard that lives on the f64->f16 path (line 8585). For `ISD::STRICT_FP_ROUND` with src f64 and dst bf16, it calls `expandRoundInexactToOdd` (non-strict graph) then emits non-strict `ISD::FP_ROUND` -- silently dropping the strict chain (operand 0), losing the second result (chain), and dropping exception semantics from the inexact-to-odd expansion. Downstream side-effecting ops can reorder past the strict round. Sibling to m137 (lowering drops payload) and m141 (bf16 family). |
447+
| [m150-s-barrier-init-m0-mask-discarded](known-miscompiles/m150-s-barrier-init-m0-mask-discarded/NOTES.md) | ❌ | ❌ | ❌ | `SIISelLowering.cpp:12450-12459` (lowering of `amdgcn.s.barrier.init` / `amdgcn.s.barrier.signal.var`) computes `M0Val = (CntOp & 0x3F)` then IMMEDIATELY overwrites with `M0Val = (CntOp << 16)` using the UNMASKED `CntOp`. The mask is dead code; the intended `(CntOp & 0x3F) << 16` expression is never produced. If member-count has bits >=6 set (e.g. 0x40, 0x80), those bits land in M0[27:22], corrupting unrelated fields when the HW decodes M0. Adjacent (not filed): `SOPInstructions.td:507-573` -- barrier pseudos lack `mayLoad/mayStore`, combined with `IntrNoMem` on intrinsics, MachineScheduler can reorder memops across `s_barrier`. |
447448
| [m149-preallocate-wwm-skips-av-class-gfx950](known-miscompiles/m149-preallocate-wwm-skips-av-class-gfx950/NOTES.md) | ❌ | ❌ | ❌ | `SIPreAllocateWWMRegs.cpp:102` (`processDef`) early-exits when `TRI->isVGPR(*MRI, Reg)` is false. `isVGPR` calls `isVGPRClass`, which is true only when a regclass has VGPRs AND no AGPRs. On gfx90A+ (gfx950 included), MAI-capable register classes are unified AV_* super-classes (`hasVGPRs && hasAGPRs`), so `isVGPRClass == false` and the WWM pre-allocator silently drops the virtreg. The physreg is never added to `WWMReservedRegs`, so VGPRAllocator may reuse it across `EXIT_STRICT_WWM` -- post-EXIT writes under restored EXEC corrupt inactive-lane data WWM was supposed to preserve. Same defect in `SILowerWWMCopies.cpp:135` (`addToWWMSpills`). Reachable via `amdgcn.strict.wwm` on MFMA results. |
448449
| [m148-v2f64-to-v2f16-gcnpat-double-rounds](known-miscompiles/m148-v2f64-to-v2f16-gcnpat-double-rounds/NOTES.md) | ❌ | ❌ | ❌ | `VOP3Instructions.td:1461-1463` GCNPat for `(v2f16 (fpround v2f64:$src))` emits `V_CVT_F32_F64` + `V_CVT_PK_F16_F32` -- classic IEEE double-rounding (f64->f32->f16). The scalar `fptrunc double to half` path (`SIISelLowering.cpp:8599` -> `LowerF64ToF16Safe`, `AMDGPUISelLowering.cpp:3787-3873`) emulates single-step rounding via inexact-to-odd correction. Same IR, same gfx950: scalar vs lane-0-of-vector fpround produce different f16 bits for f64 values near half-way f16 boundaries. Also bypasses the m137 NaN-payload preservation logic. `lowerFP_ROUND` at line 8573 bails on non-f32 source for v2f16 dst, letting the buggy GCNPat fire. Sibling of m137. |
449450
| [m147-performclampcombine-drops-snan-quietening](known-miscompiles/m147-performclampcombine-drops-snan-quietening/NOTES.md) | ❌ | ❌ | ❌ | `performClampCombine` (`SIISelLowering.cpp:18284-18303`) constant-folds `AMDGPUISD::CLAMP(c)` and returns the input sNaN bit-pattern unchanged when DX10Clamp is OFF (default for compute kernels). `AMDGPUISD::CLAMP` lowers via `ClampPat` (`SIInstructions.td:2030-2036`) to `V_MAX_F32_e64 src, src, DSTCLAMP.ENABLE`; with IEEE_MODE=1 the HW `v_max_f32(sNaN, sNaN)` quiets the sNaN (sets mantissa bit 22) but preserves payload. Constant-fold path returns `0x7F800001` (raw sNaN); HW path returns `0x7FC00001` (quieted). Reachable via `amdgcn.fmed3(c, 0.0, 1.0)`, `fminnum(fmaxnum(c, 0.0), 1.0)`, or `fmed3` pattern-matchers. Sibling family of m133. |
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# m150: `s_barrier_init` / `s_barrier_signal_var` M0 lowering discards mask immediately after computing it
2+
3+
*Discovery method: code inspection (during amdgcn.s.barrier audit).*
4+
5+
## The bug
6+
7+
`amdgpu/third_party/llvm-project/llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12450-12459`
8+
(in the lowering of `amdgcn.s.barrier.init` and
9+
`amdgcn.s.barrier.signal.var`):
10+
11+
```cpp
12+
// Intent (per the comments at 12448-12449): pack BarrierID into
13+
// M0[5:0] and member-count into M0[21:16].
14+
15+
M0Val = SDValue(DAG.getMachineNode(AMDGPU::S_AND_B32, ..., CntOp,
16+
DAG.getTargetConstant(0x3F, ...)), 0); // (1)
17+
constexpr unsigned ShAmt = 16;
18+
M0Val = DAG.getNode(ISD::SHL, DL, MVT::i32, CntOp, // (2)
19+
DAG.getShiftAmountConstant(ShAmt, MVT::i32, DL));
20+
```
21+
22+
Line (1) computes `CntOp & 0x3F` and assigns to `M0Val`, then line
23+
(2) **immediately overwrites `M0Val`** with `CntOp << 16` -- using
24+
the **unmasked** `CntOp`. The intended `(CntOp & 0x3F) << 16`
25+
expression is never produced; line (1) is dead code.
26+
27+
If the user passes a member-count with bits >=6 set (e.g. 0x40,
28+
0x80, 0x100), those bits land in M0[27:22], corrupting unrelated
29+
fields when the hardware decodes M0.
30+
31+
## Reproducer
32+
33+
`reduced.ll`:
34+
35+
```llvm
36+
declare void @llvm.amdgcn.s.barrier.init(i32, i32)
37+
38+
define amdgpu_kernel void @t(i32 %cnt) {
39+
call void @llvm.amdgcn.s.barrier.init(i32 0, i32 %cnt)
40+
ret void
41+
}
42+
```
43+
44+
With `%cnt = 0x40`:
45+
* Buggy lowering: `M0 = (0x40 << 16) = 0x400000` -> `M0[22] = 1`
46+
(corrupts the field bordering the member-count field).
47+
* Intended: `M0 = ((0x40 & 0x3F) << 16) = 0` (just the barrier ID).
48+
49+
`llc -mtriple=amdgcn -mcpu=gfx950 -O2 reduced.ll`: the emitted
50+
`S_MOV_B32 m0, ...` sequence corresponds to `CntOp << 16`, not
51+
`(CntOp & 0x3F) << 16`.
52+
53+
## Suggested fix
54+
55+
Replace the two-step assignment with a proper compose:
56+
57+
```cpp
58+
M0Val = SDValue(DAG.getMachineNode(AMDGPU::S_AND_B32, DL, MVT::i32,
59+
CntOp,
60+
DAG.getTargetConstant(0x3F, DL, MVT::i32)),
61+
0);
62+
M0Val = DAG.getNode(ISD::SHL, DL, MVT::i32, M0Val, // <-- shift the MASKED value
63+
DAG.getShiftAmountConstant(16, MVT::i32, DL));
64+
M0Val = DAG.getNode(ISD::OR, DL, MVT::i32, M0Val, BarID); // <-- OR with barrier ID
65+
```
66+
67+
## Adjacent findings (not filed separately)
68+
69+
Same audit identified two further defects in this area:
70+
71+
* `SOPInstructions.td:507-573, 1651-1664` -- `S_BARRIER`,
72+
`S_BARRIER_WAIT`, `S_BARRIER_{SIGNAL,INIT,JOIN}_{IMM,M0}` /
73+
`S_WAKEUP_BARRIER_*` pseudos set `hasSideEffects = 1` and
74+
`isConvergent = 1` but inherit `mayLoad = 0; mayStore = 0`.
75+
Combined with `IntrNoMem` on the intrinsics
76+
(`IntrinsicsAMDGPU.td:282-328`), MachineScheduler /
77+
PostRAScheduler can reorder independent `global_load` / `flat_load`
78+
/ LDS ops across `s_barrier`, breaking the synchronization.
79+
Compare `S_WAKEUP` (`SOPInstructions.td:1685-1686`) which sets both.
80+
`SIInsertWaitcnts` does not compensate.
81+
82+
* `SOPInstructions.td:2318-2335, 2917-2922` -- split-barrier real
83+
encodings exist only for gfx12/gfx13. On gfx950 the split-barrier
84+
intrinsics select to pseudos with no real opcode (MC failure)
85+
rather than being rejected up front at the intrinsic-lowering
86+
layer.
87+
88+
## Why the fuzzer hasn't caught it
89+
90+
* The IR fuzzer rarely emits `amdgcn.s.barrier.init` /
91+
`amdgcn.s.barrier.signal.var` with a runtime-variable member
92+
count. Per `MEMORY.md` (Prefer-random-over-idioms), the random
93+
emitter should generate these intrinsics with non-zero member
94+
counts including values >0x3F.
95+
* The differential O0-vs-O2 oracle would not catch this directly
96+
(both opt levels share the same lowering). An asm-pattern oracle
97+
comparing M0 setup against the documented bit layout would.
98+
99+
## Toolchain Results
100+
101+
| Toolchain | Result |
102+
| --- | --- |
103+
| LLVM HEAD with the local PR patches (`build/llvm-fuzzer`) | Lowering present; mask is dead code. |
104+
| ROCm 7.1.1 | Same defect. |
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
; m150: SIISelLowering.cpp:12450-12459 - M0 packing for
2+
; amdgcn.s.barrier.init / amdgcn.s.barrier.signal.var discards the
3+
; mask immediately after computing it.
4+
;
5+
; The intent (per the comments at lines 12448-12449): pack BarrierID
6+
; into M0[5:0] and member-count into M0[21:16]. The code:
7+
;
8+
; M0Val = SDValue(DAG.getMachineNode(AMDGPU::S_AND_B32, ..., CntOp,
9+
; getTargetConstant(0x3F, ...)), 0); // (1)
10+
; constexpr unsigned ShAmt = 16;
11+
; M0Val = DAG.getNode(ISD::SHL, DL, MVT::i32, CntOp, // (2)
12+
; getShiftAmountConstant(ShAmt, MVT::i32, DL));
13+
;
14+
; Line (1) computes (CntOp & 0x3F) and assigns to M0Val, then line
15+
; (2) immediately OVERWRITES M0Val with (CntOp << 16) using the
16+
; UNMASKED CntOp. The intended `(CntOp & 0x3F) << 16` is never
17+
; produced.
18+
;
19+
; If the user passes a member-count with bits >=6 set (e.g. 0x40,
20+
; 0x100, etc.), those bits land in M0[27:22] -- corrupting the
21+
; barrier ID / unrelated M0 fields when the HW decodes M0.
22+
;
23+
; The bug is in the SDAG lowering, so it is independent of which
24+
; concrete target ultimately consumes the synthesized M0 value. The
25+
; intrinsic is currently used by gfx12+ split-barriers, but the
26+
; defect lives in the lowering shared with future gfx950+ revisions.
27+
;
28+
; This reproducer triggers the buggy lowering by calling
29+
; amdgcn.s.barrier.init with a non-zero member-count and inspecting
30+
; the resulting M0 setup in asm.
31+
32+
source_filename = "m150-s-barrier-init-m0-mask-discarded"
33+
target triple = "amdgcn-amd-amdhsa"
34+
35+
declare void @llvm.amdgcn.s.barrier.init(i32, i32)
36+
37+
define amdgpu_kernel void @t(i32 %cnt) {
38+
; CntOp = 0x40 -> bit 6 set (out of mask).
39+
; Buggy lowering: M0 = (0x40 << 16) = 0x400000 -> M0[22] = 1 (corrupt)
40+
; Intended: M0 = ((0x40 & 0x3F) << 16) = 0 (just barrier ID)
41+
call void @llvm.amdgcn.s.barrier.init(i32 0, i32 %cnt)
42+
ret void
43+
}

0 commit comments

Comments
 (0)