Skip to content

Force Simple protocol for gfx12 single-node collectives to fix deadlock - #2187

Open
tcgu-amd wants to merge 1 commit into
ROCm:develop_deprecatedfrom
tcgu-amd:fix/gfx12-ll-protocol-deadlock-5480
Open

tcgu-amd wants to merge 1 commit into
ROCm:develop_deprecatedfrom
tcgu-amd:fix/gfx12-ll-protocol-deadlock-5480

Conversation

@tcgu-amd

Copy link
Copy Markdown

Summary

rcclUpdateCollectiveProtocol() calls rcclGetProtoForGfx12() for single-node AllReduce/AllGather/ReduceScatter on gfx12, which can select NCCL_PROTO_LL for messages under the size thresholds (~890KB for AllReduce, ~96KB for AllGather/ReduceScatter). src/device/prims_ll.h never received the gfx12 memory-ordering fixes that src/device/prims_simple.h got (introduced for gfx12 atomic acquire semantics), so selecting LL on gfx1201 deadlocks during multi-GPU collectives.

P2P send/recv already disables LL for gfx12 explicitly (src/enqueue.cc:1012); this closes the one remaining gap in the collective path by forcing NCCL_PROTO_SIMPLE instead of calling rcclGetProtoForGfx12().

This is functionally equivalent to setting NCCL_PROTO=Simple at runtime (the confirmed workaround), but bakes the fix into the build so downstream users don't need to set the env var.

Fixes #5480 (reported as an RCCL deadlock during vLLM TP=2 inference on dual R9700 / gfx1201).

Root cause

Introduced by 9545ae0 ("Navi4 LL enablement and tuning"), which added rcclGetProtoForGfx12() and removed the blanket LL disable for gfx12 in the tuning table, without adding the corresponding gfx12 device-side fixes to the LL primitives.

Test plan

  • Reproduce the original vLLM TP=2 deadlock on dual gfx1201 hardware without this patch
  • Confirm the deadlock no longer occurs with this patch applied (without needing NCCL_PROTO=Simple)
  • Confirm no regression for gfx950/gfx942 LL paths (unchanged by this patch)

Longer term, LL should be re-enabled for gfx12 once prims_ll.h gets the same gfx12 atomic-ordering treatment as prims_simple.h — this patch is a safe stopgap, not a permanent fix.

…llectives

rcclUpdateCollectiveProtocol() calls rcclGetProtoForGfx12() for
single-node AllReduce/AllGather/ReduceScatter on gfx12, which can
select NCCL_PROTO_LL for messages under ~890KB/~96KB. LL primitives
(src/device/prims_ll.h) never received the gfx12 memory-ordering
fixes that Simple primitives (src/device/prims_simple.h) got, so
selecting LL on gfx1201 deadlocks during multi-GPU collectives
(observed via vLLM TP=2 on dual R9700).

P2P send/recv already disables LL for gfx12 explicitly
(src/enqueue.cc:1012); this closes the one remaining gap in the
collective path. Equivalent to setting NCCL_PROTO=Simple, but baked
into the build so it doesn't need to be set at runtime.

Issue: ROCm/rocm-systems#5480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants