Repository navigation
test(rccl): host microtests for DDA IPC AllReduce dispatch, gates and flat/tree choice - #13240
Merged
Merged
Conversation
…tch and flat/tree selection
…ank floor, zero threshold
…taging Assert the tree path by "not the flat kernel" instead of naming the tree instantiation, and check the pre-launch scratch copy at 8 MiB, above any in-kernel staging cap. The 256 KiB threshold stays spelled out in the test. JIRA ID: AICOMRCCL-2776, AICOMRCCL-2777, AICOMRCCL-2778, AICOMRCCL-2781
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
…tsMicroDdaIpc With clang 23 and later, -fprofile-instr-generate makes a HIP TU that defines kernels register per-kernel profile shadows from its module ctor: __hipRegisterVar plus __llvm_profile_offload_register_*. Under -no-hip-rt nothing defines __hipRegisterVar, and the profile calls pull the ROCm part of libclang_rt.profile, whose interception and sanitizer_common dependencies are not linked, so the binary failed to link on ROCm 10.1. Define all three next to the other registration fakes; the profile hooks are weak and are unused on older compilers. JIRA ID: AICOMRCCL-2776, AICOMRCCL-2777, AICOMRCCL-2778, AICOMRCCL-2781
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: the tree-path assertions in dda-allreduce-ipc-test.cc only rule out the flat kernel, so half of the flat/tree choice this PR exists to pin is left unpinned.
Comment @argus review to re-run, or @argus re-review after pushing.
Contributor
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: the tree kernel is still pinned only as EXPECT_NE(Flat<T>(), ...) at dda-allreduce-ipc-test.cc:276, so a wrong NRANKS or hasAcc instantiation passes.
Comment @argus review to re-run, or @argus re-review after pushing.
… block cap The dispatch tests never read the kernel's ipcbuffs argument or the stream, so passing comm->ddaScratch as the peer table or a null stream to the launch or the scratch copy went unnoticed. They now record both and run on a sentinel stream. A failing scratch copy must return ncclUnhandledCudaError without a launch, and ncclAllReduceDdaIpcBlocks gets a case for element size and the 24-block cap. The unused union interceptor macro is gone; the hooked list now expands into the hook signature asserts, so a new hooked name without a matching seam fails to compile. Drops unused includes and a comment that cited a staging cap develop does not have. JIRA ID: AICOMRCCL-2776, AICOMRCCL-2777, AICOMRCCL-2778, AICOMRCCL-2781
Contributor
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: the tree kernel is still pinned only as EXPECT_NE against the flat one at dda-allreduce-ipc-test.cc:283, so a wrong NRANKS, dtype or hasAcc instantiation passes.
Comment @argus review to re-run, or @argus re-review after pushing.
sdarisala
approved these changes
Oct 12, 2026
aduforest
added a commit
that referenced
this pull request
Oct 15, 2026
clang >= 23 registers per-kernel profile shadows under -fprofile-instr-generate, which pulled InstrProfilingPlatformROCm into rccl-UnitTestsMicroDdaColl and broke its link in the Multi-Arch build. Weak no-op stubs, as in #13240. Co-Authored-By: Claude <noreply@anthropic.com>
…ift-dda-allreduce-host-tests # Conflicts: # projects/rccl/test/host/CMakeLists.txt # projects/rccl/test/host/MICROTEST_README.md # projects/rccl/test/host/fakes/hip_fakes.cc # projects/rccl/test/host/run_host_tests.sh # projects/rccl/tools/scripts/test_runner/configs/ci-precheckin.json
Assert the ddaAllReduceTreeIpc<T, kDdaNranks, false> address wherever the tree arm is expected, so a wrong NRANKS, dtype or hasAcc instantiation fails. Record grid.x and block.x in the hipExtLaunchKernel seam and check bf16 and fp32 at count 4096, so the element size passed to the launch geometry is observed.
Contributor
Author
|
@argus re-review |
dhanavikram
approved these changes
Oct 18, 2026
aduforest
added a commit
that referenced
this pull request
Oct 20, 2026
) JIRA ID: AICOMRCCL-2826 ## What Host-only microtests for the 15 DDA collective launchers (`src/algorithms/dda/{all_gather,alltoall,reduce_scatter}/dda_*.cu`, `all_reduce/dda_all_reduce_fabric{,_ll,_ll128}.cu`) and their LL/LL128 slot-sizing helpers, in a new `rccl-UnitTestsMicroDdaColl`. `dda_all_reduce_ipc.cu` is left to #13240. ## How - Each `.cu` is `#include`d with `hipExtLaunchKernelGGL` renamed to a recorder (`DdaCollSeams.h`), so tests check the kernel, grid, block, stream, stop event and every typed argument; kernels compile as host stubs with registration faked. - The rccl_wrap.cc DDA thresholds are test-local seams, and the copied param defaults are pinned to rccl_wrap.cc with `rccl_assert_source_line`. The binary links separately; #13116 defines same-named hooks. - Pins one bug as `Eligible_KnownBug_ScratchCheckUsesPerRankBytesNotGatheredTotal` (AllGather fabric and IPC): eligibility checks one rank's bytes against scratch, while the launcher needs `nRanks` times that. #13239 fixes it and flips both pins. ## Coverage 100% lines on 10 of the 15 launchers and all 6 slot-sizing headers; `dda_all_reduce_fabric_ll.cu` 96.5% and the four reduce-scatter launchers 97.4-98.1% (unreachable `blocks == 0` and `default:` arms, and a `% 16` check the per-rank check implies). ## Mutation Testing 75 targeted mutations: 68 killed, 7 equivalent (same clamped or rounded value, the implied `% 16` check, an `ll128Thresh == 0` test that `bytes > 0` covers). ## Test Plan Standalone build: `rccl-UnitTestsMicroDdaColl` 271 tests pass, `--gtest_shuffle --gtest_repeat=20` clean. --------- Co-authored-by: Claude <noreply@anthropic.com>
Contributor
TheRock Submodule Bump ActivityNewest first
|
This was referenced Oct 21, 2026
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.
Problem: ncclAllReduceDdaIpc's eligibility gates, its fp32/fp16/bf16 dispatch and its flat/tree kernel choice have no tests, so a swapped half/bf16 instantiation, a dropped gate or a moved 256 KiB threshold passes CI.
Fix: add the host-only rccl-UnitTestsMicroDdaIpc binary, which includes the hipified dda_all_reduce_ipc.cu and records each launch through a new hipExtLaunchKernel seam in hip_fakes.
Problem: rcclDdaEnabled's tests use zeroed gfx942 comms, so a truthy launchOrderImplicit check, dropping gfx950 or its 8-rank floor, and a zero threshold admitting 0-byte calls all pass.
Fix: add five wrap-test cases for the unset launch order, gfx950 at and below the rank floor, gfx1250 with 2 ranks and a zero threshold.
JIRA ID: AICOMRCCL-2776, AICOMRCCL-2777, AICOMRCCL-2778, AICOMRCCL-2781