Skip to content

fix(rccl): CPX 64-rank comm-init crashes (bootstrap AllGather + WarpSpeed) - #8077

Merged
Kapil-Shyam-Pawar merged 2 commits into
developfrom
users/kpawar/cpx-fix
Jul 2, 2026
Merged

Kapil-Shyam-Pawar merged 2 commits into
developfrom
users/kpawar/cpx-fix

Conversation

@Kapil-Shyam-Pawar

@Kapil-Shyam-Pawar Kapil-Shyam-Pawar commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

On gfx950 CPX+NPS2 (64 ranks), all_gather_perf aborts during comm init even after #7766.

Issues identified:

  • A bad-pointer Bad address in the topology bootstrap AllGather
NCCL WARN ncclOsSocketProgressOpt: Call to send to --- failed : Bad address
  • A segfault from WarpSpeed over-allocating channels.
NCCL INFO WarpSpeed enabled: warpSpeedChannelMultiplier 4, maxNchannels 1760, nc 1760

Technical Details

  • bootstrap.cc: ring slice offset slice * size overflowed 32-bit int with the ~102 MB per-rank topology buffer at 64 ranks
  • connect.cc: single-node WarpSpeed channel count was unclamped (nc=1760), overrunning comm->channels[MAXCHANNELS]

JIRA ID

JIRA ID : ROCM-27078

Test Plan

  • Run all_gather_perf on gfx950 CPX+NPS2 (64 GPUs)
  • Add host unit tests for the 64-bit offset and the channel clamp

Test Result

  • AllGather on CPX+NPS2 passes end-to-end with no bad address/segfault
  • New unit tests (BootstrapBidir.SliceOffsetIsComputedIn64Bit, Rcclwrap.WarpSpeedChannelClampCapsAtHalfMaxChannels) pass

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
🌿 Branch Name ✅ Pass —
📝 PR Title/Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ❌ Fail Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rccl/src/bootstrap.cc, projects/rccl/src/graph/connect.cc, projects/rccl/test/WarpSpeedMPITests.cpp; no test file found
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ Unit Test

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

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

@therock-pr-bot

therock-pr-bot Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@Kapil-Shyam-Pawar Kapil-Shyam-Pawar changed the title fix(rccl): CPX 64 ranks comm-init crashes (bootstrap AllGather + WarpSpeed channels) fix(rccl): CPX 64-rank comm-init crashes (bootstrap AllGather + WarpSpeed) Jul 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses two comm-init crashes seen on gfx950 CPX+NPS2 at 64 ranks: (1) a bootstrap AllGather “Bad address” caused by 32-bit overflow in slice-offset pointer arithmetic, and (2) a WarpSpeed single-node channel over-allocation that can overrun the fixed comm->channels[MAXCHANNELS] array.

Changes:

  • Compute ring slice indices as size_t in bootstrap bidirectional AllGather paths to ensure slice * size is evaluated in 64-bit.
  • Clamp WarpSpeed single-node channel count to MAXCHANNELS/2 in ncclTopoPostset to prevent comm->channels[] overflow.
  • Add two host-side regression tests intended to guard the above issues.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
projects/rccl/src/bootstrap.cc Switches bidirectional ring AllGather slice indices to size_t to avoid 32-bit offset overflow.
projects/rccl/src/graph/connect.cc Caps single-node WarpSpeed channel count at MAXCHANNELS/2 to prevent channel array overruns.
projects/rccl/test/BootstrapBidirTests.cpp Adds a host regression test for 64-bit slice-offset arithmetic (currently not exercising production code).
projects/rccl/test/RcclWrapTests.cpp Adds a host regression test for WarpSpeed channel clamping (currently not exercising production code).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread projects/rccl/test/RcclWrapTests.cpp Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@Kapil-Shyam-Pawar
Kapil-Shyam-Pawar marked this pull request as ready for review July 1, 2026 21:17
@Kapil-Shyam-Pawar
Kapil-Shyam-Pawar requested a review from a team as a code owner July 1, 2026 21:17

@i-kosarev i-kosarev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Kapil-Shyam-Pawar
Kapil-Shyam-Pawar merged commit 831fad2 into develop Jul 2, 2026
17 of 19 checks passed
@Kapil-Shyam-Pawar
Kapil-Shyam-Pawar deleted the users/kpawar/cpx-fix branch July 2, 2026 22:36
marbre added a commit that referenced this pull request Jul 7, 2026
…peed) (#8077) (#8194)

Cherry-picks commit 831fad2 into the 7.14 release branch.

---

## Motivation

On gfx950 CPX+NPS2 (64 ranks), all_gather_perf aborts during comm init
even after #7766.

Issues identified:
 - A bad-pointer `Bad address` in the topology bootstrap AllGather 
```
NCCL WARN ncclOsSocketProgressOpt: Call to send to 10.235.24.26<60509> failed : Bad address
```
 - A segfault from WarpSpeed over-allocating channels.
```
NCCL INFO WarpSpeed enabled: warpSpeedChannelMultiplier 4, maxNchannels 1760, nc 1760
```
## Technical Details

- bootstrap.cc: ring slice offset slice * size overflowed 32-bit int
with the ~102 MB per-rank topology buffer at 64 ranks
- connect.cc: single-node WarpSpeed channel count was unclamped
(nc=1760), overrunning comm->channels[MAXCHANNELS]

## JIRA ID

JIRA ID : ROCM-27078

## Test Plan

- Run all_gather_perf on gfx950 CPX+NPS2 (64 GPUs)
- Add host unit tests for the 64-bit offset and the channel clamp

## Test Result

- AllGather on CPX+NPS2 passes end-to-end with no bad address/segfault
- New unit tests (`BootstrapBidir.SliceOffsetIsComputedIn64Bit`,
`Rcclwrap.WarpSpeedChannelClampCapsAtHalfMaxChannels`) pass

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

Co-authored-by: Kapil S. Pawar <KapilShyam.Pawar@amd.com>
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.

4 participants