Repository navigation
feat(rccl): add fp8 (e4m3, e5m2) support to DDA collectives - #13311
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
|
@cppccppccppc @sunguobao PTAL |
1 similar comment
|
@cppccppccppc @sunguobao PTAL |
mustafabar
left a comment
mustafabar
left a comment
There was a problem hiding this comment.
LGTM, however, as suggested below, please restrict the support to gfx1250. We can extend support to previous platforms in a future PR so that this is smaller and more self-contained.
Also, your reduction is happening at dword granularity and it could be done better using pk8 instructions. I suspect the ROI maybe limited for smaller messages, so we can consider this for a future PR. My PR applies the pk8 instructions on the vanilla path #13415
There was a problem hiding this comment.
🟡 Changes recommended
The new tests have unresolved kernel-registration, MPI failure-coordination, and cached-tier isolation defects.
3 open issues
What changed in this PR
Adds both fp8 formats to RCCL’s DDA collectives across IPC, fabric VMM, LL, and LL128 tiers.
Changes:
- Centralizes datatype eligibility and adds fp8 reduction dispatch.
- Implements packed fp8 addition using stable kernel tags.
- Adds eligibility, arithmetic, and MPI integration tests.
| File | Description |
|---|---|
| projects/rccl/test/DdaIpcEligibilityTests.cpp | Tests IPC fp8 eligibility and dispatch. |
| projects/rccl/test/DdaFp8MPITests.cpp | Adds fabric-tier fp8 integration tests. |
| projects/rccl/test/DdaFabricEligibilityTests.cpp | Tests fabric fp8 eligibility and dispatch. |
| projects/rccl/test/DdaCollCommonTests.cpp | Adds exhaustive fp8 addition checks. |
| projects/rccl/test/common/DdaFp8TestHelpers.hpp | Shares eligibility and dispatch test helpers. |
| projects/rccl/test/CMakeLists.txt | Includes fp8 MPI test sources. |
| projects/rccl/src/include/rccl_common.h | Centralizes supported DDA datatypes. |
| projects/rccl/src/algorithms/dda/reduce_scatter/dda_reduce_scatter_ipc.cu | Enables IPC fp8 reduction dispatch. |
| projects/rccl/src/algorithms/dda/reduce_scatter/dda_reduce_scatter_fabric.cu | Enables VMM fp8 reduction dispatch. |
| projects/rccl/src/algorithms/dda/reduce_scatter/dda_reduce_scatter_fabric_ll128.cu | Enables LL128 fp8 reduction dispatch. |
| projects/rccl/src/algorithms/dda/reduce_scatter/dda_reduce_scatter_fabric_ll.cu | Enables LL fp8 reduction dispatch. |
| projects/rccl/src/algorithms/dda/device/CollCommon.h | Adds stable fp8 tags and packed addition. |
| projects/rccl/src/algorithms/dda/device/CollCommon_ll128.h | Documents fp8 payload packing. |
| projects/rccl/src/algorithms/dda/alltoall/dda_alltoall_ipc.cu | Admits fp8 IPC copies. |
| projects/rccl/src/algorithms/dda/alltoall/dda_alltoall_fabric.cu | Admits fp8 VMM copies. |
| projects/rccl/src/algorithms/dda/alltoall/dda_alltoall_fabric_ll128.cu | Admits fp8 LL128 copies. |
| projects/rccl/src/algorithms/dda/alltoall/dda_alltoall_fabric_ll.cu | Admits fp8 LL copies. |
| projects/rccl/src/algorithms/dda/all_reduce/dda_all_reduce_ipc.cu | Enables IPC fp8 reduction dispatch. |
| projects/rccl/src/algorithms/dda/all_reduce/dda_all_reduce_fabric.cu | Enables VMM fp8 reduction dispatch. |
| projects/rccl/src/algorithms/dda/all_reduce/dda_all_reduce_fabric_ll.cu | Enables fp8 LL/LL128 reduction variants. |
| projects/rccl/src/algorithms/dda/all_gather/dda_all_gather_ipc.cu | Admits fp8 IPC copies. |
| projects/rccl/src/algorithms/dda/all_gather/dda_all_gather_fabric.cu | Admits fp8 VMM copies. |
| projects/rccl/src/algorithms/dda/all_gather/dda_all_gather_fabric_ll128.cu | Admits fp8 LL128 copies. |
| projects/rccl/src/algorithms/dda/all_gather/dda_all_gather_fabric_ll.cu | Admits fp8 LL copies. |
Review effort: Balanced
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
There was a problem hiding this comment.
ARGUS
Findings
5 more finding(s), not posted inline
projects/rccl/test/DdaFp8MPITests.cpp:127(Low) DdaFp8MPI fixtures rely on a comment for one-process-per-fixture; RCCL_PARAM caching fails later fixtures in an unfiltered runprojects/rccl/test/DdaCollCommonTests.cpp:37(Low) Fp8*AddMatchesCoreForAllPairs compares hadd2 with hadd2 on every arch except gfx942/gfx1250, so rounding parity is only checked thereprojects/rccl/test/DdaFabricEligibilityTests.cpp:1550(Low) DdaDatatypeSupportTest.AdmitsFloatTypesAndBothFp8Formats restates the rcclDdaDatatypeSupported switch line for lineprojects/rccl/src/algorithms/dda/all_reduce/dda_all_reduce_fabric_ll128.cu:154(Low) Uncalled ncclAllReduceDdaFabricLL128Eligible_old / _old dispatcher still carry the old fp32/fp16/bf16 list In dda_all_reduce_fabric_ll128.cu (outside the diff),ncclAllReduceDdaFabricLL128Eligible_oldat line 154 andncclAllReduceDdaFabricLL128_oldat lines 185-198 keep the pre-rcclDdaDatatypeSupportedtype list and have no callers in the tree. Worth a follow-up to delete them so a stale copy of the list does not linger.projects/rccl/test/DdaFp8MPITests.cpp:73(Low) isGfx1250Device() is now copied verbatim in three DDA MPI test files
This PR changes 1,123 lines across 24 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.
Bottom line: Worth settling before approval: fp8AddFloatsKernel is instantiated on rccl_float8 / rccl_bfloat8, so the ExactSums tests cannot find their kernel on gfx942 runners where DdaCollCommon.* runs.
Comment @argus review to re-run, or @argus re-review after pushing.
3ca2769 to
7caa136
Compare
- Admit fp8 e4m3/e5m2 only on gfx1250 (incl. strict) via rcclDdaDatatypeSupported(comm, datatype); revert the IPC tier (gfx942/gfx950) to develop. - Reduce fp8 with the pk8 hadd8/hadd8_b intrinsics, 8 bytes per op, in the LL, LL128, and VMM fabric kernels; drop the hadd2 path. - Sum peers in global rank order in the LL and LL128 one-shot AllReduce kernels so every rank produces identical results. - Tests: tag-templated fp8 kernels with exhaustive lane checks against hadd2/hadd2_b, arch-gate eligibility tests, and a DdaFabricMPITest-based fp8 MPI fixture with per-tier categories. Co-authored-by: Cursor <cursoragent@cursor.com>
7caa136 to
54b226e
Compare
|
Thanks. Both are done in this PR: fp8 is admitted only on gfx1250 (including strict), and the reductions use the pk8 helpers from #13415. gfx942/gfx950 will follow separately. |
- Move the all-pairs DDA fp8 add check into the existing DdaVecElementAdd suite, on DeviceTestBase's buffer and launch helpers, and restore DdaCollCommonTests.cpp to develop. - Drop the DDA exact-sums tests: they call the same hadd8 / hadd8_b that Fp8ReduceTest.ExactSmallSums* already pins. - Share isGfx1250() on DeviceTestBase instead of a per-file copy in TestFp8Reduce.cpp. - Add Fp8ReduceTest.* to the standard fixtures category. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
ARGUS
This PR changes 1,211 lines across 28 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.
Bottom line: Worth settling before approval: rcclDdaDatatypeSupported sends fp8 on gfx1250 to DDA by default, and there is no fp8-specific opt-in (rccl_wrap.cc:1212). The fp8 MPI tests are also missing from the gfx1250 runner configs.
Comment @argus review to re-run, or @argus re-review after pushing.
#13311 already folds the LL and LL128 one-shot AllReduce in rank order. Take develop's all_reduce_dda_ll.h as is. In all_reduce_dda_ll128.h keep develop's file and replace only the reduce loop with the one that loads the own slice first and seeds from rank 0, which keeps the NRANKS_CT=8 float and half kernels out of VGPR spills (develop: 128 VGPRs, 27 spilled).
#13311 moved the DDA fabric launchers' datatype check into rcclDdaDatatypeSupported() in rccl_wrap.cc and added fp8 dispatch to the reducing launchers. Seam the gate (default mirrors rccl_wrap.cc), check each fabric eligibility defers to it, and pin the fp8 e4m3/e5m2 kernels. Co-Authored-By: Claude <noreply@anthropic.com>
TheRock Submodule Bump ActivityNewest first
|
Motivation
The DDA collectives (AllReduce, ReduceScatter, AllGather, AlltoAll) accepted only fp32, fp16 and bf16, so fp8 traffic fell back to the ring/tree kernels even at sizes where DDA serves the other types. This adds
ncclFloat8e4m3andncclFloat8e5m2to the DDA fabric tiers (VMM, LL, LL128) on gfx1250. Other architectures, and the IPC tier (gfx942/gfx950 only), keep rejecting fp8 and are left for a follow-up.Technical Details
rcclDdaDatatypeSupported(comm, datatype)inrccl_wrap.cc. It admits both fp8 types only whencomm->archNamematchesgfx1250, a prefix match that also coversgfx1250strict.int8_t, so only the gate changes.dda::common::fp8e4m3/fp8e5m2.rccl_float8/rccl_bfloat8are not used as template arguments because they name different types in the host pass and the gfx942 device pass (FNUZ vs OCP), which would register kernels under names the gfx942 code object lacks.hadd8/hadd8_bhelpers from perf(rccl): pk8 fp8/bf8 sum reduction on gfx1250 #13415 (v_cvt_scale_pk8_f32_*+v_cvt_scalef32_pk8_*_f32), 8 bytes per call, through a newvecElementAddFp8x8. The LL payload add, the LL128 word add, and the 16-byte VMM add all go through it. On other architectures the fp8 instantiations compile to a trap; the gate keeps them from being launched.(selfRank + r) % nRanksorder, so ranks rounded differently and could disagree in the last bit. They now add all ranks in global rank order, so every rank computes the same result. This applies to all datatypes.Issue Tracking
JIRA ID: AICOMRCCL-2915
Test Plan
On a single node with 4x gfx1250 (UALoE fabric), RCCL built with
--amdgpu_targets gfx1250:-g 4 -c 1 -A 1), out-of-place and in-place:RCCL_DDA_LL_ONESHOT_THRESHOLD=0(LL two-shot)RCCL_DDA_LL_THRESHOLD=0(LL128)RCCL_DDA_LL_THRESHOLD=0 RCCL_DDA_LL128_ONESHOT_THRESHOLD=0(LL128 two-shot)RCCL_DDA_LL=0 RCCL_DDA_THRESHOLD=67108864(fabric VMM)RCCL_DDA_ENABLE=0.DdaVecElementAdd.Fp8*): rccl-tests picks inputs whose sums are exact in any order, so it validates data movement but not rounding. A kernel feeds all 65,536 fp8 byte pairs throughddaLLAddPayloadandddaLL128AddWord(8 lanes each) and the 16-bytevecElementAdd(16 lanes), and compares every result bit-for-bit with core RCCL'shadd2/hadd2_b, for both formats. It skips on non-gfx1250 devices. Thehadd8/hadd8_bmath itself, including the E8M0 scale, is covered by perf(rccl): pk8 fp8/bf8 sum reduction on gfx1250 #13415'sFp8ReduceTest.*, which this PR adds to the standard fixtures category.DdaFabricEligibilityTest.*,DdaFabricLL128EligibilityTest.*): fp8 is admitted on gfx1250 and rejected on gfx950 / gfx942 and for a null comm, for every fabric gate.-t 4 -g 1 -G 10 -n 20 -w 5 -c 1), 1 KB to 64 MB, default vsRCCL_DDA_ENABLE=0. fp8_e4m3 for all four collectives, plus bf16 AllReduce and ReduceScatter as a control. Three interleaved rounds; each figure is the median. A single thread driving all four GPUs (-g 4) serializes their launches and hides DDA's advantage for every datatype, so it is not used.Test Result
rccl-tests: 0 wrong in every run. The
-A 1column shows DDA LL, LL128, and SIMPLE (VMM setting) serving fp8 in both formats; the AlltoAll rows above the DDA cap fall back to Direct.Exhaustive add check: 0 mismatches for e4m3 and e5m2 in all three helpers. Swapping the e4m3 and e5m2 pk8 adds makes it fail at every width.
Fp8ReduceTest.*: 15/15 pass.Eligibility tests: pass. Two AllGather threshold tests (
AllGatherLL_PastThresholdRejected,AllGatherLL128_PastThresholdRejected) fail the same way ondevelop.gfx1250 disassembly of the fp8 kernels contains only pk8 conversions.
Performance: time saved by DDA relative to
RCCL_DDA_ENABLE=0, as1 - DDA time / non-DDA time, out-of-place. In-place is within a few points.Rounds varied by less than 5% in most cells (worst 19%, fp8 AllReduce at 16 MB). Up to 1 MB fp8 gains as much as bf16. From 4 MB up it gains less, and at 64 MB fp8 AllReduce and AllGather are about even with the non-DDA path; tuning the fp8 thresholds is left for a follow-up. AlltoAll above 1 MB is outside DDA's cap on both sides. e5m2 was not timed; it runs the same kernels with a different conversion instruction.
Not covered:
DdaFp8MPITests(no MPI on this node), and fp8 AllGather/AlltoAll in e5m2 or under the forced tier settings.Submission Checklist