[AMDGPU] Fix cross-half shuffle select for top-half lanes on wave64#769
[AMDGPU] Fix cross-half shuffle select for top-half lanes on wave64#769ZJLi2013 wants to merge 1 commit into
Conversation
amdgpu_cross_half_shuffle_i32 selected between the same-half and other-half ds_bpermute reads with (target_lane ^ self_lane) & 32. Because the ds_bpermute byte address masks the lane to 5 bits, from_self_half always resolves to the low SIMD32 (lanes 0..31) and from_other_half (via the permlane64 swap) always resolves to the high SIMD32 (lanes 32..63), independent of the executing lane. The correct payload therefore depends only on which half the target lane lives in. The old self-dependent select was wrong for lanes 32..63: for a target in the same half it picked the other-half read, so shuffle_up on the upper half returned lane (target_lane - 32) instead of target_lane, breaking Hillis-Steele scans on wave64. Select on target_lane & 32. Validated on R9700 (gfx1201 / RDNA4): identity and scan primitives pass.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9aecef049
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| i32 from_self_half = amdgpu_ds_bpermute(byte, value); | ||
| i32 from_other_half = amdgpu_ds_bpermute(byte, swapped); | ||
| return ((target_lane ^ self_lane) & 32) ? from_other_half : from_self_half; | ||
| return (target_lane & 32) ? from_other_half : from_self_half; |
There was a problem hiding this comment.
On gfx10/RDNA1-2, which this backend still supports via the has_permlane64 == false LDS fallback in llvm_context.cpp, ds_bpermute in wave64 operates within each 32-lane half rather than as a wave-global low-half read. With this unconditional target-half select, a top-half lane doing an in-half read (for example identity shuffle with self_lane == target_lane == 40) selects from_other_half, so it reads the swapped payload for lane 8 instead of lane 40; top-half cross-half reads are inverted as well. This needs to keep the self-relative selection for those targets or be gated by the actual ds_bpermute semantics.
Useful? React with 👍 / 👎.
2 participants
Summary
On wave64 AMD GPUs, every cross-half subgroup shuffle (
subgroup.shuffle,shuffle_up,shuffle_down) returns the wrong value on the upper half of the wave (lanes 32..63).amdgpu_cross_half_shuffle_i32picks between the twods_bpermutereads with a self-relative condition, but the reads are absolute, so the select is inverted for the top-half lanes.The most visible consequence is that
subgroup.inclusive_*_tiled/exclusive_*_tiled(Hillis-Steele scans, built onshuffle_up) miscompile from lane 32 onward on a full wave64.reduce_*_tiled/shuffle_downhappen to only consume the low-half result, so they escape — which is why this went unnoticed.One-line fix: select on the target lane's half only.
Root cause
amdgpu_cross_half_shuffle_i32issues two reads and selects per lane:Because
bytemasks the lane index to 5 bits, on wave64 the reads are absolute, not lane-relative:from_self_halfalways resolves to the low SIMD32 (lanes 0..31),from_other_half(via thepermlane64swap) always resolves to the high SIMD32 (lanes 32..63),regardless of which half the executing lane sits in. The correct payload therefore depends only on which half the target lane lives in —
target_lane & 32— and must not depend on the executing lane.The old
(target_lane ^ self_lane) & 32is correct for the bottom half (self_lane & 32 == 0) but inverts the choice for the top half: for a same-half target it picks the other-half read. Concretely, identityshuffle(v, i)returnsvalue[i & 31]on lanes 32..63 (e.g. lane 40 reads lane 8), andshuffle_upon the upper half returns lanetarget_lane - 32instead oftarget_lane.Fix
self_lanebecomes unused and is removed.Testing
Built the runtime bitcode from source and validated with standalone quadrants kernels (no Genesis), single wave64,
log2_size=6.R9700 (gfx1201 / RDNA4) — the affected path:
shuffle(v, i)value[i&31])inclusive_add_tiled(6)exclusive_add_tiled(6)reduce_add_tiled(6)MI300 (gfx942 / CDNA3) — not affected, no regression: on CDNA the
ds_bpermutelane mask is the full 64 andpermlane64is patched to identity, sofrom_self_half == from_other_halfand the select is a no-op. Identity / scan / reduce PASS both before and after this change.Notes / scope
ds_bpermuteaddressing absolute lanes 0..31 for the masked low-31 address on wave64 — confirmed on gfx1201 (RDNA4). If any older RDNA part (gfx10.x) is genuinely SIMD32-relative here, it would need a separate path; I don't have that hardware to check and have flagged it rather than assumed it.permlane64codegen crash via a per-archds_bpermutelane mask). [AMDGPU] Don't emit llvm.amdgcn.permlane64 on CDNA #746 leaves this RDNA half-select bug in place; the two changes are complementary.