Repository navigation
perf(rccl): default gfx1250 collectives to a 192-thread block - #14564
Merged
Merged
Conversation
A 192-thread block is faster than 256 on the gfx1250 paths that use the collective block size. Across Broadcast, AllReduce, AllGather, and ReduceScatter at 1 to 8 nodes, excluding DDA sizes, the geomean latency improvement is about 7%. RCCL_THREADS_PER_BLOCK still overrides the default, and 256 remains a valid block size.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ARGUS
Findings
1 more finding(s), not posted inline
projects/rccl/src/enqueue/enqueue.cc:3364(Low) Direct RS copy of the nt sequence drops the 3-warp clamp from line 3240; keep it for parity.
Bottom line: Worth settling before approval: the gfx1250 192 default overrides a lower NCCL_NTHREADS and also applies to unmeasured SPX, and the Direct RS hunk only runs on gfx950, where it changes env handling.
Comment @argus review to re-run, or @argus re-review after pushing.
Contributor
pr-rocm-systems-14564-oci-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 61 tests skipped. |
Contributor
pr-rocm-systems-14564-ruby64-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 189 tests skipped. pr-rocm-systems-14564-ruby64-rccl-pr-smokeBuilds
Failures
15 tests skipped. |
A lower NCCL_NTHREADS was being raised to 192, and the default also applied to unmeasured SPX. Cap only the DPX table at 192, and leave gfx950 Direct ReduceScatter on its previous thread count.
Contributor
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: the gfx1250 DPX early return skips the PAT override, so a PAT collective launches 96 threads and hangs. Also, the RCCL_THREADS_PER_BLOCK test never builds a DPX comm, and nothing tests the gfx1250 arch guard.
Comment @argus review to re-run, or @argus re-review after pushing.
Contributor
pr-rocm-systems-14564-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14564-ruby64-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
PAT's maxThreads entry is unset on HIP, so the 192-thread cap turned a 0 into a 96-thread launch and the PAT compute thread never ran. Leave PAT on the full block, and test that a shared gfx950 table does not take the DPX cap.
Contributor
pr-rocm-systems-14564-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14564-ruby64-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
mustafabar
approved these changes
Nov 2, 2026
alex-breslow-amd
approved these changes
Nov 3, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A 192-thread block is faster than 256 on the gfx1250 paths that use the collective block size. Across Broadcast, AllReduce, AllGather, and ReduceScatter at 1 to 8 nodes, excluding DDA sizes, the geomean latency improvement is about 7%. RCCL_THREADS_PER_BLOCK still overrides the default, and 256 remains a valid block size.
Motivation
On gfx1250, Ring LL, LL128, and Simple all launch with a 256-thread block. Scale-up measurements show a 192-thread block, six waves of 32, is faster on the collectives that use that launch size. The gain shows up from 1 to 8 nodes on Broadcast, AllReduce, AllGather, and ReduceScatter once DDA sizes are set aside, since DDA keeps its own block size. The 256-thread cap stays in place so an explicit RCCL_THREADS_PER_BLOCK=256 still works.
Technical Details
RCCL_GFX1250_DEFAULT_NTHREADS is 192. In rcclOptThreadBlockSize, a gfx1250 communicator takes that value and returns before the older per-protocol logic, so LL, LL128, and Simple all get 192. Previously a single-node collective was forced to RCCL_SINGLE_NODE_MAX_NTHREADS (256), multi-node LL was forced to RCCL_LL_MAX_NTHREADS (256), and LL128 and Simple used maxThreads, also 256. RCCL_THREADS_PER_BLOCK is still applied first and is still clamped to the 256-thread maximum. Channel selection is unchanged and still uses maxThreads. Direct ReduceScatter was resetting nWarps from maxThreads after that choice, so it now calls rcclOptThreadBlockSize as well. DDA, CE, and symmetric kernels do not go through this function.
Issue Tracking
JIRA ID: AICOMRCCL-2941
Test Plan
Run broadcast_perf, all_reduce_perf, all_gather_perf, and reduce_scatter_perf at 1, 2, 4, and 8 nodes on gfx1250, 8 GPUs per node (DPX mode), from 1 KB to 16 GB. Compare the unmodified library with the 192-thread library and drop any size whose algorithm is DDA. OptThreadBlockSize_Gfx1250DefaultIs192 checks that 1-node Ring Simple and 2-node Ring LL both return 192. OptThreadBlockSize_Gfx1250UserOverrideKeeps256 checks that RCCL_THREADS_PER_BLOCK=256 still launches 256.
Test Result
On gfx1250 every kept row had #wrong 0. The geomean latency improvement across the non-DDA sizes is 7.2%. By collective it is Broadcast 7.3%, AllReduce 7.1%, AllGather 11.1%, and ReduceScatter 3.9%. Per node count: Broadcast is 2.6%, 13.4%, 10.7%, and 2.8% at 1, 2, 4, and 8 nodes; AllReduce is 5.4%, 7.7%, 8.0%, and 7.7%; AllGather is 10.1%, 14.1%, 11.4%, and 9.0%; ReduceScatter is 4.2%, 3.5%, 4.5%, and 3.5%.
Submission Checklist