[PERF][distributed]: optimize deterministic ROCm collectives with HIP IPC - #357
Open
maxiaosong1124 wants to merge 1 commit into
Conversation
Signed-off-by: maxiaosong1124 <maxiaosong7890@outlook.com>
maxiaosong1124
requested review from
EthanZero2Hero,
Flink-ddd,
KJLdefeated,
bitborne and
inaniloquentee
as code owners
August 29, 2026 10:07
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
frank-2077
added a commit
to frank-2077/RL-Kernel
that referenced
this pull request
Aug 29, 2026
Port the HIP IPC fixed-tree transport and packed reduce-scatter path from PR RL-Align#357, with RCCL fallback and focused ROCm coverage.
frank-2077
added a commit
to frank-2077/RL-Kernel
that referenced
this pull request
Aug 29, 2026
Record the PR RL-Align#357 collective measurements, preserve the four-way same-topology comparison, and publish the refreshed MI300X artifacts.
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.
Summary
Stacked on #356, this PR replaces the ROCm transport-only hot path with a single-node HIP IPC transport while preserving the same fixed balanced rank tree.
reduce_scatter_manyso FFN sequence-parallel backward can reduce the independent gate/up lanes under one ready/done generation without changing either lane's expression tree.Design
Every rank owns one
hipMallocstaging allocation. Handles are exchanged once and imported with HIP IPC. A system-scope release/acquire generation publishes the local input and waits for every peer; a second done generation prevents staging reuse while a peer can still read it. Close uses a third peer acknowledgement before releasing the allocation.Reduction arithmetic remains:
FP16/BF16 use two-element vector instructions only when the output is aligned. Scalar tails and offset views use the scalar kernel. Empty AllGather inputs complete without launching a zero-sized grid.
The MI300X routing policy is:
Determinism and correctness
Validation
41 passedin the CPU/reference, ROCm transport, and build-isolation suite.3 passedin the 8-GPU MI300X AllReduce/AllGather/ReduceScatter cross-TP suite.git diff --checkpassed.Benchmark
Environment and method:
all_rank_repeat_bitwise=true.Lower latency is better. The two comparison columns express the latency change of this PR; “faster” means lower latency.
Against #356, 10 of 12 rows improve, with 44%–88% lower latency on the improved reduction rows. Against native RCCL, every 4 KiB and 64 KiB row is 24%–39% faster, and 1 MiB ReduceScatter is 19% faster. Large strict deterministic reductions remain slower than native RCCL because native RCCL does not guarantee the fixed floating-point reduction order required by this implementation. The only material regression against #356 is the 1 MiB AllReduce crossover (+14.13% latency); the 1 MiB and 16 MiB AllGather differences are within noise.
Compute/communication fusion
This PR does not claim asynchronous overlap or compute/communication fusion. The API remains stream ordered and reports
supports_async_overlap = Falseandsupports_compute_communication_fusion = False.