[AMDGPU] Don't emit llvm.amdgcn.permlane64 on CDNA#746
Conversation
v_permlane64_b32 is an RDNA-only instruction: it exists on gfx11 (RDNA3) and gfx12 (RDNA4) but on no CDNA part. quadrants previously enabled the llvm.amdgcn.permlane64 intrinsic on gfx940/gfx941/gfx942 (CDNA3) as well. On gfx942 the AMDGPU backend does not cleanly "Cannot select" the intrinsic -- it selects the V_PERMLANE64_B32 pseudo, which has no valid MC opcode for CDNA, and then crashes with a bare SIGSEGV in SIInstrInfo::getInstSizeInBytes during the branch-relaxation pass. This made scene.build() segfault for any kernel using a cross-half subgroup shuffle (genesis-world #2962). Gate has_permlane64 to gfx11/gfx12 only, so every CDNA target (and gfx10.x RDNA1/2) takes the existing LDS-roundtrip software emulation, which produces correct cross-half results on wave64 hardware. Also drop the QD_AMDGPU_FORCE_PERMLANE64_FALLBACK env-var escape hatch and correct the related comments in llvm_context.cpp, runtime.cpp and test_simt.py.
Gating permlane64 off CDNA (prev commit) stops the SIGSEGV but exposed a latent correctness bug: the cross-half shuffle helper is RDNA-shaped. It masks ds_bpermute's target to 31 and relies on permlane64 to fetch the top half, which is correct only where ds_bpermute is SIMD32-scoped (RDNA). On CDNA ds_bpermute is wave64-wide, so masking to 31 means every lane reads a bottom-half lane and the top half is never reached -- shuffle_xor(v,32) returned [32..63, 32..63] instead of [32..63, 0..31] on gfx942 (CI never caught this: RDNA uses the native instruction, not this path). Make the lowering architecture-aware via two JIT-patched knobs: - amdgpu_ds_bpermute_lane_mask(): 63 on GCN/CDNA (gfx9xx, wave64-wide ds_bpermute), 31 on RDNA (SIMD32-scoped). With mask 63 a single wide ds_bpermute already returns lane target_lane for the whole wave. - permlane64 patched to the identity on CDNA, so the helper's cross-SIMD branch equals the same-SIMD branch and the per-lane select is a true no-op (and the intrinsic, which has no MC opcode on CDNA, is never emitted). RDNA paths are unchanged: gfx11/gfx12 keep native v_permlane64_b32, gfx10.x keeps the LDS-roundtrip emulation, both with lane mask 31.
The subgroup docs described emitting v_permlane64_b32 on CDNA as "well-defined and free", which is exactly the pre-fix behavior that crashed the AMDGPU backend (genesis-world #2962). Document the actual per-arch lowering: a single wave-wide ds_bpermute on CDNA (no permlane64) and the permlane64 + ds_bpermute + select pairing on RDNA wave64.
|
Validated this on real CDNA3 hardware — it fixes the gfx942 SIGSEGV. Please consider merging. Setup. AMD MI300X (gfx942 / CDNA3), host ROCm 6.3.2, Result (minimal Franka
The only variable is whether quadrants carries this PR, so the fix is confirmed at the codegen level. Why it matters beyond that one case. genesis-world 1.1.1 worked around the crash by rewriting one kernel (Franka self-collision) to use Minor build notes (clean releated to Genesis-Embodied-AI/genesis-world#2962 |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
I skim-read through the user-facing doc changes, and look reasonable to me:
|
|
|
||
| - `value` and `offset` dtypes: same as `shuffle` above; `offset` is a `u32`. | ||
| - Maps to `__shfl_down_sync` on CUDA and `OpGroupNonUniformShuffleDown` on SPIR-V. On AMDGPU it is emulated with `ds_bpermute`; wave64 cross-half offsets (any `offset >= 32` for low-half lanes, or any non-zero `offset` for high-half lanes that lands across the SIMD32 boundary) go through the same `permlane64 + ds_bpermute + select` lowering as `shuffle` - see [AMDGPU wave64 cross-half lowering](#amdgpu-wave64-cross-half-lowering). These operations are added on both RDNA and CDNA. | ||
| - Maps to `__shfl_down_sync` on CUDA and `OpGroupNonUniformShuffleDown` on SPIR-V. On AMDGPU it is emulated with `ds_bpermute`; wave64 cross-half offsets (any `offset >= 32` for low-half lanes, or any non-zero `offset` for high-half lanes that lands across the SIMD32 boundary) go through the same wave64 cross-half lowering as `shuffle` - a single wave-wide `ds_bpermute` on CDNA, or a `permlane64 + ds_bpermute + select` sequence on RDNA - see [AMDGPU wave64 cross-half lowering](#amdgpu-wave64-cross-half-lowering). These operations are added on both RDNA and CDNA. |
There was a problem hiding this comment.
All this doc is very advanced. I guess it is not meant for the average user. Like, I don't think an average end-user like me care about the fact that it maps to __shfl_down_sync or whatever. I just want to know if it is highly optimised on all devices, and whether I should expect lower performance on older gen hardware, stuffs like this. Personally I cannot do anything with the information that is implemented as a permlane64 + ds_bpermute + select sequence on RDNA.
It seems that you have been following this pattern extensively in this documentation. And know that I'm taking a step back, I'm wondering if all of this is on the right track. Like, there should be a general high-level document explaining what users can expect from this submodule, but then, the documentation for every specific method should sit on the docstring of each specific method directly, not centralised on some markdown general doc.
Anyway, it is out of scope of this PR, but you should have this in mind and track it somewhere.
There was a problem hiding this comment.
Yeah, I agree with your first paragraph.
And yes, probably should be handled in a separate PR from this one. Created ticket in internal tracker, as CMP-328
There was a problem hiding this comment.
Seems this fails the new doc quality check anyway... so I'll probably have to create a PR to fix subgroup.md first, before merging this PR.
Address the "Check doc quality" reviewer flags on subgroup.md: define IR at first use, and drop internal C++ symbols, the fe-ll cache key, the segmented_reduce bitmask-path walkthrough, and the ballot_first_n LLVM isel-bug rationale from user-facing sections, keeping the user-observable behaviour and cost.
Give shuffle / shuffle_up / shuffle_down / broadcast / invocation_id / sync / mem_fence proper docstrings (behaviour, dtype rules, backend lowering, caller contract), so per-method documentation lives on the function itself and is rendered by autoapi. Preparatory step for slimming subgroup.md down to a high-level overview.
…eference" This reverts commit 372ac4c.
… check" This reverts commit 4ec69fe.
…64-on-cdna # Conflicts: # docs/source/user_guide/subgroup.md
hughperkins
added
awaiting-ci-to-pass
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
3 participants
v_permlane64_b32 is an RDNA-only instruction: it exists on gfx11 (RDNA3) and
gfx12 (RDNA4) but on no CDNA part. quadrants previously enabled the
llvm.amdgcn.permlane64 intrinsic on gfx940/gfx941/gfx942 (CDNA3) as well. On
gfx942 the AMDGPU backend does not cleanly "Cannot select" the intrinsic -- it
selects the V_PERMLANE64_B32 pseudo, which has no valid MC opcode for CDNA, and
then crashes with a bare SIGSEGV in SIInstrInfo::getInstSizeInBytes during the
branch-relaxation pass. This made scene.build() segfault for any kernel using a
cross-half subgroup shuffle (genesis-world #2962).
Gate has_permlane64 to gfx11/gfx12 only, so every CDNA target (and gfx10.x
RDNA1/2) takes the existing LDS-roundtrip software emulation, which produces
correct cross-half results on wave64 hardware. Also drop the
QD_AMDGPU_FORCE_PERMLANE64_FALLBACK env-var escape hatch and correct the
related comments in llvm_context.cpp, runtime.cpp and test_simt.py.Issue: #
Brief Summary
copilot:summary
Walkthrough
copilot:walkthrough