Repository navigation
fix(rccl): Make device API / GIN examples build on ROCm - #11579
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
There was a problem hiding this comment.
🟢 Approval recommended
Remaining documentation nits are minor and non-blocking.
Pull request overview
Adds ROCm build support for the NCCL Device API and GIN examples using hipification and RCCL-aware linking.
Changes:
- Adds category-level build, test, install, and clean targets.
- Adds build-time hipify rules and RCCL/CMake build-tree discovery.
- Updates example Makefiles and ignores generated artifacts.
File summaries
| File | Summary |
|---|---|
projects/rccl/examples/makefiles/rccl_examples.mk |
Shared ROCm, RCCL, MPI, and hipify build rules. |
projects/rccl/examples/06_device_api/Makefile |
Category-level orchestration. |
projects/rccl/examples/06_device_api/03_alltoall_hybrid/c/Makefile |
Hybrid example ROCm build integration; README documentation nit noted. |
projects/rccl/examples/06_device_api/02_alltoall_gin/c/Makefile |
GIN example ROCm build integration; README documentation nit noted. |
projects/rccl/examples/06_device_api/01_allreduce_lsa/c/Makefile |
LSA example ROCm build integration; README documentation nit noted. |
projects/rccl/examples/.gitignore |
Ignores generated staging files and binaries. |
Review details
Suppressed comments (2)
projects/rccl/examples/06_device_api/02_alltoall_gin/c/Makefile:9
- The corresponding
c/README.mdstill tells users to build withNCCL_HOME/CUDA_HOMEand describes this as a CUDA/NCCL example, but this Makefile now consumesRCCL_HOME/ROCM_PATHand invokeshipify-perl/hipcc. Please update the README so the documented command actually configures this ROCm build.
include ../../../makefiles/rccl_examples.mk
projects/rccl/examples/06_device_api/03_alltoall_hybrid/c/Makefile:9
- The corresponding
c/README.mdstill tells users to build withNCCL_HOME/CUDA_HOMEand describes this as a CUDA/NCCL example, but this Makefile now consumesRCCL_HOME/ROCM_PATHand invokeshipify-perl/hipcc. Please update the README so the documented command actually configures this ROCm build.
include ../../../makefiles/rccl_examples.mk
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
ARGUS
Findings
1 finding(s) that could not be posted as inline comments
projects/rccl/examples/06_device_api/02_alltoall_gin/c/main.cu:152(Low) make test in the new category Makefile exits 0 on a host without GIN, having run no collective06_device_apimake testreports success on a host without GIN.02_alltoall_gin/c/main.cu:149printsERROR: ... does not support GIN!and02_alltoall_gin/c/main.cu:152returns NULL without running the collective, thenexamples/common/src/utils.cc:261passes NULL topthread_joinso that result is discarded and the process exits 0. Both files sit outside this diff, so this looks like follow-up material rather than a change here.
Bottom line: LGTM. The include repoint, hipify staging and RCCL_HOME search order all check out; the two notes are follow-ups, not changes for this PR.
Comment @argus review to re-run, or @argus re-review after pushing.
|
|
## Motivation The `06_device_api` examples were synced from NCCL but never wired up for ROCm, so they could not be built at all ## Technical Details - Added the missing `examples/06_device_api/Makefile` - Pointed the three leaf `c/Makefiles` at `rccl_examples.mk` - the old include path resolved outside the repo and pulled in CUDA settings - Added a build-time `hipify-perl` step, so the upstream `.cu` sources stay byte-identical to NCCL and future syncs apply cleanly. - Search `RCCL_HOME` before `ROCM_PATH`, and accept a CMake build tree as `RCCL_HOME`. - Ignore the built binaries and hipify staging directory. ## Issue Tracking JIRA ID: AICOMRCCL-1529 ## Test Plan Run from `examples/06_device_api`: ``` export RCCL_HOME=<path/to/rccl/build/release> export MPI_HOME=<path/to/mpi/install> export GPU_TARGETS=<gfx_arch> export NCCL_CUMEM_ENABLE=1 NCCL_GIN_TYPE=2 # 1. LSA allreduce cd 01_allreduce_lsa/c && make clean && make && \ $MPI_HOME/bin/mpirun -np 8 ./allreduce_lsa # 2. Pure GIN alltoall cd 02_alltoall_gin/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_gin # 3. Hybrid GIN+LSA alltoall cd 03_alltoall_hybrid/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_hybrid # 4. All three via the category Makefile make clean && make test MPI=1 ``` ## Test Result All three examples build and pass ``` AllReduce completed. Result verification: PASSED Pure GIN AlltoAll result: PASSED Hybrid AlltoAll result: PASSED ``` ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
TheRock Submodule Bump ActivityNewest first
|
## Motivation The `06_device_api` examples were synced from NCCL but never wired up for ROCm, so they could not be built at all ## Technical Details - Added the missing `examples/06_device_api/Makefile` - Pointed the three leaf `c/Makefiles` at `rccl_examples.mk` - the old include path resolved outside the repo and pulled in CUDA settings - Added a build-time `hipify-perl` step, so the upstream `.cu` sources stay byte-identical to NCCL and future syncs apply cleanly. - Search `RCCL_HOME` before `ROCM_PATH`, and accept a CMake build tree as `RCCL_HOME`. - Ignore the built binaries and hipify staging directory. ## Issue Tracking JIRA ID: AICOMRCCL-1529 ## Test Plan Run from `examples/06_device_api`: ``` export RCCL_HOME=<path/to/rccl/build/release> export MPI_HOME=<path/to/mpi/install> export GPU_TARGETS=<gfx_arch> export NCCL_CUMEM_ENABLE=1 NCCL_GIN_TYPE=2 # 1. LSA allreduce cd 01_allreduce_lsa/c && make clean && make && \ $MPI_HOME/bin/mpirun -np 8 ./allreduce_lsa # 2. Pure GIN alltoall cd 02_alltoall_gin/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_gin # 3. Hybrid GIN+LSA alltoall cd 03_alltoall_hybrid/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_hybrid # 4. All three via the category Makefile make clean && make test MPI=1 ``` ## Test Result All three examples build and pass ``` AllReduce completed. Result verification: PASSED Pure GIN AlltoAll result: PASSED Hybrid AlltoAll result: PASSED ``` ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
## Motivation The `06_device_api` examples were synced from NCCL but never wired up for ROCm, so they could not be built at all ## Technical Details - Added the missing `examples/06_device_api/Makefile` - Pointed the three leaf `c/Makefiles` at `rccl_examples.mk` - the old include path resolved outside the repo and pulled in CUDA settings - Added a build-time `hipify-perl` step, so the upstream `.cu` sources stay byte-identical to NCCL and future syncs apply cleanly. - Search `RCCL_HOME` before `ROCM_PATH`, and accept a CMake build tree as `RCCL_HOME`. - Ignore the built binaries and hipify staging directory. ## Issue Tracking JIRA ID: AICOMRCCL-1529 ## Test Plan Run from `examples/06_device_api`: ``` export RCCL_HOME=<path/to/rccl/build/release> export MPI_HOME=<path/to/mpi/install> export GPU_TARGETS=<gfx_arch> export NCCL_CUMEM_ENABLE=1 NCCL_GIN_TYPE=2 # 1. LSA allreduce cd 01_allreduce_lsa/c && make clean && make && \ $MPI_HOME/bin/mpirun -np 8 ./allreduce_lsa # 2. Pure GIN alltoall cd 02_alltoall_gin/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_gin # 3. Hybrid GIN+LSA alltoall cd 03_alltoall_hybrid/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_hybrid # 4. All three via the category Makefile make clean && make test MPI=1 ``` ## Test Result All three examples build and pass ``` AllReduce completed. Result verification: PASSED Pure GIN AlltoAll result: PASSED Hybrid AlltoAll result: PASSED ``` ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
## Motivation The `06_device_api` examples were synced from NCCL but never wired up for ROCm, so they could not be built at all ## Technical Details - Added the missing `examples/06_device_api/Makefile` - Pointed the three leaf `c/Makefiles` at `rccl_examples.mk` - the old include path resolved outside the repo and pulled in CUDA settings - Added a build-time `hipify-perl` step, so the upstream `.cu` sources stay byte-identical to NCCL and future syncs apply cleanly. - Search `RCCL_HOME` before `ROCM_PATH`, and accept a CMake build tree as `RCCL_HOME`. - Ignore the built binaries and hipify staging directory. ## Issue Tracking JIRA ID: AICOMRCCL-1529 ## Test Plan Run from `examples/06_device_api`: ``` export RCCL_HOME=<path/to/rccl/build/release> export MPI_HOME=<path/to/mpi/install> export GPU_TARGETS=<gfx_arch> export NCCL_CUMEM_ENABLE=1 NCCL_GIN_TYPE=2 # 1. LSA allreduce cd 01_allreduce_lsa/c && make clean && make && \ $MPI_HOME/bin/mpirun -np 8 ./allreduce_lsa # 2. Pure GIN alltoall cd 02_alltoall_gin/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_gin # 3. Hybrid GIN+LSA alltoall cd 03_alltoall_hybrid/c && make clean && make MPI=1 && \ $MPI_HOME/bin/mpirun -np 8 ./alltoall_hybrid # 4. All three via the category Makefile make clean && make test MPI=1 ``` ## Test Result All three examples build and pass ``` AllReduce completed. Result verification: PASSED Pure GIN AlltoAll result: PASSED Hybrid AlltoAll result: PASSED ``` ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
Motivation
NCCL 2.30.7 added the RMSNorm kernel fusion examples (examples/07_kernel_fusion: LSA, Multimem, GIN, Hybrid LSA/GIN). They came into RCCL with the sync but still included the upstream makefiles/{common,examples}.mk (outside the repo) and built with nvcc, so none of them could be built on ROCm. This PR wires them up the same way #11579 did for 06_device_api.
Validating them also exposed a defect in the public device API headers that affects every application kernel using the GIN device API, not only these examples. This PR fixes it as well (see Device API fix below).
Technical Details
The four leaf c/Makefiles include examples/makefiles/rccl_examples.mk. The .cu sources are hipified at build time with hipify-perl into .hipify/ and stay byte-identical to NCCL, so future syncs apply cleanly.
rccl_examples.mk: new $(HIPIFY_DIR)/%.cuh rule. The shared include/rmsnorm_utils.cuh includes <cuda_runtime.h>, so it is hipified too. The quoted #include in the hipified main.cu searches .hipify/ first and picks up the HIP copy.
The leaf READMEs use the ROCm build variables (RCCL_HOME, ROCM_PATH, GPU_TARGETS), and the Makefile help targets drop the CUDA requirements, as in 06_device_api.
examples/README.md lists 07_kernel_fusion, and its build line covers 01..07, matching examples/Makefile.
The four binaries are added to examples/.gitignore.
Motivation
The
06_device_apiexamples were synced from NCCL but never wired up for ROCm, so they could not be built at allTechnical Details
examples/06_device_api/Makefilec/Makefilesatrccl_examples.mk- the old include path resolved outside the repo and pulled in CUDA settingshipify-perlstep, so the upstream.cusources stay byte-identical to NCCL and future syncs apply cleanly.RCCL_HOMEbeforeROCM_PATH, and accept a CMake build tree asRCCL_HOME.Issue Tracking
JIRA ID: AICOMRCCL-1529
Test Plan
Run from
examples/06_device_api:Test Result
All three examples build and pass
Submission Checklist