Skip to content

Add Multi-node RCCL support - #12194

Open
af-ayala wants to merge 2 commits into
developfrom
users/alaayala/rccl_multinode
Open

af-ayala wants to merge 2 commits into
developfrom
users/alaayala/rccl_multinode

Conversation

@af-ayala

@af-ayala af-ayala commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Motivation

This PR extends the optional RCCL backend to MPI plans so the transpose can use RCCL collectives (GPU-direct, Slingshot/OFI when the net plugin is present) instead of MPI. The product rule is the same as a one-GPU-per-rank distributed FFT: RCCL is used only when every rank in the plan communicator owns exactly one GPU.

Technical Details

  • RCCL is used for the multi-process transpose only when every MPI rank owns exactly one GPU (NCCL rank = sorted (mpi_rank, device); with that gate, NCCL rank equals MPI rank). Otherwise, or with ROCFFT_RCCL_DISABLE=1, the existing MPI A2A/P2P path remains.

  • Rank 0 broadcasts ncclUniqueId, each rank calls ncclCommInitRank for its local device, and CommRCCLAllToAll launches only local buffers. MPI is still used for brick metadata and this bootstrap.

Test Plan

Test locally on available AMD clusters and by CI,

Test Result

Local tests passed. CI ongoing.

Submission Checklist

JIRA ID: AIFFT-456

@therock-pr-bot

therock-pr-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 10.00000% with 63 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
projects/rocfft/library/src/plan.cpp 0.00% 39 Missing ⚠️
...jects/rocfft/library/src/include/rocfft_location.h 26.92% 17 Missing and 2 partials ⚠️
projects/rocfft/library/src/include/tree_node.h 0.00% 5 Missing ⚠️

❌ Your project check has failed because the head coverage (46.49%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12194      +/-   ##
===========================================
- Coverage    69.68%   69.32%   -0.36%     
===========================================
  Files         2803     2804       +1     
  Lines       461587   462043     +456     
  Branches     68031    68121      +90     
===========================================
- Hits        321634   320305    -1329     
- Misses      116678   118378    +1700     
- Partials     23275    23360      +85     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 38.39% <ø> (ø) Carriedforward from 42bfe77
TensileLite-Unit 76.40% <ø> (ø) Carriedforward from 42bfe77
hipBLAS 90.62% <ø> (ø) Carriedforward from 42bfe77
hipBLASLt 35.22% <ø> (ø) Carriedforward from 42bfe77
hipCUB 82.68% <ø> (ø) Carriedforward from 42bfe77
hipDNN 86.94% <ø> (ø) Carriedforward from 42bfe77
hipFFT 43.43% <ø> (ø) Carriedforward from 42bfe77
hipRAND 76.12% <ø> (ø) Carriedforward from 42bfe77
hipSOLVER 69.03% <ø> (ø) Carriedforward from 42bfe77
hipSPARSE 86.99% <ø> (ø) Carriedforward from 42bfe77
rocBLAS 48.30% <ø> (ø) Carriedforward from 42bfe77
rocFFT 46.49% <10.00%> (-4.72%) ⬇️
rocRAND 57.04% <ø> (ø) Carriedforward from 42bfe77
rocSOLVER 77.32% <ø> (ø) Carriedforward from 42bfe77
rocSPARSE 74.59% <ø> (ø) Carriedforward from 42bfe77
rocThrust 91.60% <ø> (ø) Carriedforward from 42bfe77

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
projects/rocfft/library/src/include/plan.h 24.53% <ø> (-3.77%) ⬇️
projects/rocfft/library/src/include/rocfft_mpi.h 0.00% <ø> (ø)
projects/rocfft/library/src/tree_node.cpp 37.50% <ø> (-5.18%) ⬇️
projects/rocfft/library/src/include/tree_node.h 44.85% <0.00%> (+0.97%) ⬆️
...jects/rocfft/library/src/include/rocfft_location.h 26.92% <26.92%> (ø)
projects/rocfft/library/src/plan.cpp 27.13% <0.00%> (-5.23%) ⬇️

... and 88 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@af-ayala
af-ayala marked this pull request as ready for review September 17, 2026 04:59
@af-ayala
af-ayala requested review from a team as code owners September 17, 2026 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant