Repository navigation
fix(rccl): tell CPX partitions apart in the duplicate-GPU guard - #12867
Merged
dlamd1dai merged 2 commits intoOct 10, 2026
Merged
Conversation
ncclCommInitRank refuses any multi-rank communicator on MI300X in CPX
mode with "Multiple Ranks are using the same GPU/Partition", even though
each rank is bound to a distinct partition. One rank per partition is how
CPX is meant to be run, so the default path on a supported partitioning
mode is a hard init failure, and the message misdescribes the cause.
The guard compares gpuUuid, which comes from hipDeviceGetUuid, which
returns ROCr's HSA_AMD_AGENT_INFO_UUID, which is KFD's unique_id. That id
belongs to the physical device, so all 8 CPX partitions of an OAM report
one UUID and the guard reads them as one GPU. The comment above the check
asserted the opposite ("with MloPart, each partition has a different GPU
UUID"); that holds for NVIDIA MLOPart, not here.
Fix: also require busId to match. HIP reports the KFD location_id PCI
function, which is unique per partition, so partitions separate while two
ranks genuinely on one partition still match every term and are still
refused. NCCL_MULTI_RANK_GPU_ENABLE=1 was the only workaround and gives
the check up wholesale. NVIDIA MLOPart siblings share a BDF but carry
distinct UUIDs, so that path is unaffected.
Measured on dell300x-ccs-aus-k13-09 in CPX: 63 partitions report 8
distinct UUIDs, each shared by 8 partitions, and 63 distinct busIds.
JIRA ID: AICOMRCCL-2749
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
Argus flagged the MLOPart UUID sentence and the self busId note as wrong, and two nits in the new test comment. Co-authored-by: Cursor <cursoragent@cursor.com>
pvallem
approved these changes
Oct 6, 2026
Contributor
|
please get one more approval before merging. |
wenkaidu
approved these changes
Oct 10, 2026
dlamd1dai
deleted the
users/dondai/aicomrccl-2749-cpx-partition-identity
branch
October 10, 2026 01:53
anujshuk-amd
pushed a commit
that referenced
this pull request
Oct 12, 2026
## Motivation `ncclCommInitRank` refuses any multi-rank communicator on MI300X in CPX mode, even though each rank is bound to a distinct compute partition: ``` Multiple Ranks are using the same GPU/Partition. Set NCCL_MULTI_RANK_GPU_ENABLE=1 to enable this configuration. ``` One rank per partition (64 ranks on an 8-OAM node) is how CPX is meant to be run, so the default path on a supported partitioning mode is a hard init failure, with a message that misdescribes the cause — the ranks do not share a partition. `NCCL_MULTI_RANK_GPU_ENABLE=1` was the only workaround, and it disables the check wholesale, so a genuine two-ranks-on-one-partition misconfiguration also sails through. ## Technical Details The guard at `init.cc` compares `gpuUuid`, which comes from `cuDeviceGetUuid`/`hipDeviceGetUuid`. On ROCm that returns ROCr's `HSA_AMD_AGENT_INFO_UUID`, which is KFD's `UniqueID`: - `rocr-runtime/.../amd_gpu_agent.cpp:2576` formats `properties_.UniqueID` as `"GPU-%016lx"`. - `clr/rocclr/device/rocm/rocdevice.cpp:1082` strips the `GPU-` prefix and keeps the 16 ASCII characters as the 16 UUID bytes. - `clr/hipamd/src/hip_device.cpp:589` hands those bytes to `hipDeviceGetUuid`. `UniqueID` identifies the physical device, so every CPX partition of one OAM returns byte-identical bytes and the guard reads eight partitions as one GPU. The comment above the check asserted the opposite — "with MloPart, each partition has a different GPU UUID" — which holds for NVIDIA MLOPart but not here; it is rewritten. **Fix:** also require `busId` to match. HIP reports the KFD `location_id` PCI function (`hip_device_runtime.cpp:611` formats `deviceTopology_.pcie.function`, set from `hsa_bdf_id & 0x07` at `rocdevice.cpp:712`), which is unique per partition, and `busIdToInt64` keeps the nibble. The conjunct only ever narrows the guard: | case | UUID | busId | before | after | |---|---|---|---|---| | two CPX partitions of one OAM | same | differs | refused (the bug) | admitted | | two ranks on one partition | same | same | refused | refused | | NVIDIA MLOPart siblings | differs | same | admitted | admitted | No `hasMloPart` special case and no new identity source: the partition-unique value was already in the `ncclPeerInfo` the check compares. ### Not a change to GPU identity Making `hipDeviceGetUuid` partition-unique would be a ROCr/KFD behaviour change for every consumer that keys on the UUID, so it is deliberately not attempted here. ROCr's derived CUID (`HSA_AMD_AGENT_INFO_CUID`, keyed by render node) is partition-unique — each partition has its own `renderD128`-`renderD191` — but it has no public HIP getter and is in flux (#12075, #10818), so it is not a dependency worth taking for this. ### Note on a related claim The rccl-tests banner shows all partitions of an OAM under one BDF, which looks like a second instance of this defect but is not. `rccl-tests/src/util.cu:690` formats `[%04x:%02x:%02x]` from `prop.pciDomainID/pciBusID/pciDeviceID`; there is no function field in the format string and `hipDeviceProp_t` has no PCI function member. That is upstream nccl-tests behaviour and purely cosmetic. RCCL's own `busId` carries the function, measured as 63 distinct values across 63 partitions. The `busId` collapse asserted in #12742's description and in AICOMRCCL-2749 is wrong on this point; both are corrected. ## Issue Tracking JIRA ID: AICOMRCCL-2749 Found while verifying the CPX work in #12742, and listed there as follow-up 5. Independent of #12742 and #12841: it predates them and neither touches this guard. The 8-rank run in #12742 needed `NCCL_MULTI_RANK_GPU_ENABLE=1` to get off the ground; with this change it does not. ## Test Plan Host tests (no GPU), three new cases on the existing `InitMicrotest` rung: - `SameGpuUuidDifferentPciFunctions_IsAllowed` — the CPX shape; the regression test for this defect. - `SameGpuUuidSamePciFunction_StillRefused` — a real duplicate is still refused, so the fix is not the workaround in disguise. - `SamePciFunctionDifferentGpuUuids_IsAllowed` — the MLOPart shape stays admitted. `PeerSpec` gains a `busId` field; it defaults to 0, which is what the faked `getBusId` gives the self slot, so the three pre-existing guard tests are unaffected. Hardware, MI300X CPX (8 partitions per OAM, NPS1) on `dell300x-ccs-aus-k13-09`, ROCm 7.0.1 host. A and B differ by this one line only — B was relinked from the same build tree after deleting the conjunct. ## Test Result - [x] **Full host suite: 4894 passed, 0 failed** across 10 binaries (5 pre-existing `SymKernelMaskTest` TMA skips). - [x] **Mutation check:** with the conjunct deleted, `SameGpuUuidDifferentPciFunctions_IsAllowed` fails and the other two still pass. - [x] **A (before, 8 ranks on 8 CPX partitions, no `NCCL_MULTI_RANK_GPU_ENABLE`):** `ncclInvalidUsage`, `init.cc` WARN on every rank — the defect reproduces. - [x] **B (after, same command):** `Out of bounds values : 0 OK` at every size up to 64 MB, avg bus bandwidth 34.8 GB/s, exit 0. - [x] **C (after, 2 ranks forced onto partition 0 via `NCCL_TESTS_DEVICE=0`):** still refused, same `init.cc` WARN — the guard keeps its value. - [x] **Identity probe on the same node:** 63 partitions report 8 distinct UUIDs (each shared by 8 partitions) and 63 distinct busIds. In SPX the same node reports 8 distinct UUIDs and 8 distinct busIds, which is the control. Device 0's UUID bytes decode to the ASCII string `c5414a56413bd889`; KFD node 2's `unique_id` is `14213723633280080009` = `0xC5414A56413BD889`. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
TheRock Submodule Bump ActivityNewest first
|
9 tasks done
dlamd1dai
added a commit
that referenced
this pull request
Oct 18, 2026
) This PR is the **RCCL half** of the CPX GIN fix; the rocSHMEM half is #12841, now merged to `develop` as `c34c1b1f56`. `CODEOWNERS` assigns `projects/rocshmem/` to `@ROCm/rocshmem-reviewers` and `projects/rccl/` to `@ROCm/rccl-reviewers`/`@ROCm/rccl-leads`, so the change is split by owner. This half is 996 insertions across 11 files, all under `projects/rccl/`. **Rebased onto `develop` after #12841 merged.** `gin_anvil_sdma_factory.cc` calls `disconnectDevice()` and its test calls `oamMapEngineNeedsFold()` and `pciFunctionBus()`, all added by #12841, which also adds `anvil_engine_map.hpp` to RCCL's `--rocshmem-gin` copy list. That dependency is now satisfied by `develop` itself, so the branch carries only the 11 RCCL commits and no rocSHMEM files. **Replaces #12603.** Review history lives there: four rounds of Argus review ending in LGTM, with all 37 threads resolved. #12603 was opened from a fork where `RCCL PR gate` can never pass on its own. ## Summary GIN Anvil SDMA (`NCCL_GIN_TYPE=7`) failed to initialize on MI300X in CPX mode. Four defects stacked on top of each other, each masking the next. Two are in RCCL and are fixed here; the two Anvil defects were fixed in #12841. **Note on the JIRA title:** the ticket says GIN Type 6, but Anvil SDMA is `NCCL_GIN_TYPE=7` on this branch (`6` is rocSHMEM GDA). All verification used `NCCL_GIN_TYPE=7`. 1. **The RCCL GIN gate rejected every CPX partition** (`init.cc`). `fillInfo` stamps `mloPart = fn` on every CPX partition, but the `hasMloPart` carve-out only exempted partition 0, so ranks on partitions 1-7 set `comm->hasMloPart` and the gate turned GIN off. That dropped `symmetricSupport`, which routed `ncclCommWindowRegister` into `windowRegisterNonSym` and produced `hipIpcGetMemHandle failed: invalid argument`. Fix: `NCCL_GIN_MLOPART`, mirroring `NCCL_NET_GDR_MLOPART` at `graph/paths.cc:522`. The parameter is tri-state. By default a partitioned communicator keeps GIN on a single node and loses it across several: only the single-node shape has been run, and past one node admitting a partition additionally reaches the hierarchical Copy Engine path that `ncclHierCeAllowed` gates on `nNodes > 1` and `symmetricSupport` together. `1` admits partitions on any number of nodes, which is the opt-in a multi-node partitioned deployment needs; `0` restores the pre-parameter exclusion. A refused multi-node partitioned communicator names the override through `NCCL_DEBUG=INFO`, and the `GIN/RMA support` line from #11133 now reports `nNodes` beside `mloPart`. The condition is lifted into `ginGateAllows()` so it can be asserted directly. 2. **`windowRegisterNonSym` had no cuMem path** (`dev_runtime.cc`). Even with the gate fixed, the fallback only implemented legacy IPC, and GIN requires `NCCL_CUMEM_ENABLE=1`. Probe with `cuMemRetainAllocationHandle` and import through `ncclP2pImportShareableBuffer`; legacy IPC still covers genuine `cudaMalloc` buffers. Teardown branches per peer on `ipcPeerIsCuMem`. A window spanning several segments is refused outright, since the exchange publishes one handle and one size. ### Argus High findings fixed - **Resume could not re-match the import.** The import previously passed `ncclMemOffload` with the exporter's device VA as `ownerPtr`. Resume matches an imported entry against the owner-published list, which `mem_manager.cc:806` builds only from entries with `numExportedPeers > 0`, and that counter is incremented solely by `ncclP2pAllocateShareableBuffer`. `windowRegisterNonSym` exports a user buffer it never allocated, so no owner entry exists and the match at `:894` failed into `ncclSystemError`. Now imported as `ncclMemPersist` with a null owner, matching `transport/net.cc:699-701`. `ownerPtr` became dead and was removed from the record along with its two duplicated `static_assert`s. - **A failed GIN create destroyed another communicator's queues.** `gin_anvil_sdma_create`'s failure path called `anvil.disconnect()`, which clears the process-global channel map. Each communicator gets a fresh `ginState` and the connect loop deliberately reuses queues a previous create left behind, so a second create hitting the budget refusal destroyed the first's queues while their handles were already live on the GPU. Now it records the destinations it newly connected and releases only those via `disconnectDevice()`. ### Fold tests `AnvilOamMapEngine.*` asserts when the CPX fold runs, not which engine it picks. That choice is rocSHMEM's contract and differs between #12841 (even engines only) and #13237 (every engine), so it is pinned only by rocSHMEM's `anvil_engine_map_gtest`. ## Rebase onto `develop` The rebase from the old #12841 tip (`b47b4f7ccf`) onto `develop` (`c34c1b1f56`) had one conflict, in `init.cc`'s GIN/RMA log. #11133 replaced the conditional `symmetricSupport ... crossNicSupport` INFO line, which this PR had extended with `hasMloPart` and `nNodes`, with an always-on `GIN/RMA support` line that already reports `mloPart`. The resolution keeps #11133's line, adds `nNodes` to it, and keeps the `NCCL_GIN_MLOPART` multi-node hint after it. Every other file's change is identical to the pre-rebase branch. ## Test plan - [x] **Host tests on the rebased head (`fbf8a6d3c3`): 6,068 passed, 0 failed, 5 skipped** (the pre-existing `SymKernelMaskTest` skips), run through `test/host/run_host_tests.sh` in the CI image `rocm/dev-ubuntu-22.04:7.2`. Includes all `InitMicrotest.GinGate_*` and `WindowRegisterNonSym*` cases. Everything below was taken before the rebase (previous head `84a64a92dc`, before #12841 merged). Since then the only code changes on this branch are the fold-test trim above and the log-line resolution. - [x] Host tests on the split branch: 4,912 passed, 0 failed, 5 skipped across 10 binaries, with no compile errors. Run on `dell300x-ccs-aus-k13-09`. - [x] New coverage `WindowRegisterNonSymCuMemTest.VmmWindow_ExportsHandleAndImportsPeers` asserts the import is `ncclMemPersist` with null owners for both peers, plus `.RetainFails_FallsBackToLegacyIpc`, `.MultiSegmentWindow_FailsRegistrationWithoutGathering` and `.LaterSegmentAddressRangeFails_FailsRegistrationWithoutGathering`. - [x] `ginGateAllows()` asserted directly in `init-test.cc` across seven cases: an explicit `0` and `1` on both one and two nodes, the unpartitioned carve-out, and the three default cases (partitioned/one node admitted, partitioned/multi-node refused, unpartitioned/multi-node admitted). Mutating the node term to `return true` fails only `GinGate_MloPartDefaultRefusesPartitionAcrossNodes`, so the new branch is load-bearing. - [x] Gate confirmed on CPX hardware by A/B on `dell300x-ccs-aus-k13-09` (8 ranks, two OAMs, `NCCL_LSA_TEAM_SIZE=4`): `NCCL_GIN_MLOPART=1` reports `globalGinSupport 1` and `=0` reports `globalGinSupport 0`, with `hasMloPart 1` in both. `symmetricSupport` could not be exercised there: that host's driver fails the `HIP_VERSION >= 7.2.0` cuMem check, so `cuMemEnable` is 0 and `symmetricSupport` is 0 whatever the gate decides. - [x] 8-rank CPX reduce-scatter across two OAMs on `dell300x-ccs-aus-k13-09`: `Out of bounds values : 0 OK` at every size, no `hipIpcGetMemHandle` failures, no queue-create failures, no init abort. This run predates #12867 and needed `NCCL_MULTI_RANK_GPU_ENABLE=1` (see below). - [x] `install.sh -l --rocshmem-gin` builds clean (0 errors). - [x] `RCCL env-var changelog`: `NCCL_GIN_MLOPART` is described on the RCCL env-var page, so the check passes instead of asking for `[internal-env]`. - [x] The remaining `AnvilOamMapEngine.*` and `AnvilPciFunctionBus.*` cases (10 tests) compiled standalone against `anvil_engine_map.hpp` from both the #12841 tip (`b47b4f7ccf`) and #13237 (`a674a04501`): all pass on both. The removed fold-result expectations fail 5 of 8 on #12841 and pass on #13237. ## CPX partition identity (AICOMRCCL-2749): fixed by #12867 During CPX verification, `init.cc` refused a multi-rank CPX communicator with "Multiple Ranks are using the same GPU/Partition", because every CPX partition of one OAM reports the same `gpuUuid`. #12867 (merged) fixed the duplicate-GPU guard by also comparing `busId`, which carries the partition's PCI function. The 8-rank run above predates that fix. ## Follow-ups (not in this PR) 1. `NCCL_GIN_MLOPART` gate host assertion at the real call site. The init rung still cannot reach `init.cc`'s connect block: `ncclTopoComputeP2pChannels`, `rcclCommSetP2pShiftSize`, `ncclProfilerPluginInit`, `ncclTransportCheckP2pType` and `ncclProxyCreate` are fail-loud in between. 2. Multi-segment VMM window support in `windowRegisterNonSym`, which needs per-segment handles in `ExchangeEntry`. 3. RCCL-side graceful fallback when a GIN backend fails to connect, so an over-subscribed mesh disables GIN instead of failing `ncclCommInitRank`. #13235 is the rocSHMEM-side fallback and does not cover RCCL. 4. Anvil signal path indexes peers by GIN team rank instead of LSA team position (`signal nRanks=8 != devr->lsaSize=4`): #13429 (AICOMRCCL-2706). 5. `rccl-UnitTestsGinAnvilSdmaFactory` is appended to `RCCL_TEST_EXECUTABLES` but never registered anywhere that runs, unlike the sibling GPU targets at `test/CMakeLists.txt:522` and `:552`. An `add_test()` is not the fix: `test/CMakeLists.txt:42-46` records that nothing in RCCL CI runs ctest, so it would never gate. The gap predates this PR and lives outside its files. JIRA ID: AICOMRCCL-2387 --------- Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Motivation
ncclCommInitRankrefuses any multi-rank communicator on MI300X in CPX mode, even though each rank is bound to a distinct compute partition:One rank per partition (64 ranks on an 8-OAM node) is how CPX is meant to be run, so the default path on a supported partitioning mode is a hard init failure, with a message that misdescribes the cause — the ranks do not share a partition.
NCCL_MULTI_RANK_GPU_ENABLE=1was the only workaround, and it disables the check wholesale, so a genuine two-ranks-on-one-partition misconfiguration also sails through.Technical Details
The guard at
init.cccomparesgpuUuid, which comes fromcuDeviceGetUuid/hipDeviceGetUuid. On ROCm that returns ROCr'sHSA_AMD_AGENT_INFO_UUID, which is KFD'sUniqueID:rocr-runtime/.../amd_gpu_agent.cpp:2576formatsproperties_.UniqueIDas"GPU-%016lx".clr/rocclr/device/rocm/rocdevice.cpp:1082strips theGPU-prefix and keeps the 16 ASCII characters as the 16 UUID bytes.clr/hipamd/src/hip_device.cpp:589hands those bytes tohipDeviceGetUuid.UniqueIDidentifies the physical device, so every CPX partition of one OAM returns byte-identical bytes and the guard reads eight partitions as one GPU. The comment above the check asserted the opposite — "with MloPart, each partition has a different GPU UUID" — which holds for NVIDIA MLOPart but not here; it is rewritten.Fix: also require
busIdto match. HIP reports the KFDlocation_idPCI function (hip_device_runtime.cpp:611formatsdeviceTopology_.pcie.function, set fromhsa_bdf_id & 0x07atrocdevice.cpp:712), which is unique per partition, andbusIdToInt64keeps the nibble.The conjunct only ever narrows the guard:
No
hasMloPartspecial case and no new identity source: the partition-unique value was already in thencclPeerInfothe check compares.Not a change to GPU identity
Making
hipDeviceGetUuidpartition-unique would be a ROCr/KFD behaviour change for every consumer that keys on the UUID, so it is deliberately not attempted here. ROCr's derived CUID (HSA_AMD_AGENT_INFO_CUID, keyed by render node) is partition-unique — each partition has its ownrenderD128-renderD191— but it has no public HIP getter and is in flux (#12075, #10818), so it is not a dependency worth taking for this.Note on a related claim
The rccl-tests banner shows all partitions of an OAM under one BDF, which looks like a second instance of this defect but is not.
rccl-tests/src/util.cu:690formats[%04x:%02x:%02x]fromprop.pciDomainID/pciBusID/pciDeviceID; there is no function field in the format string andhipDeviceProp_thas no PCI function member. That is upstream nccl-tests behaviour and purely cosmetic. RCCL's ownbusIdcarries the function, measured as 63 distinct values across 63 partitions. ThebusIdcollapse asserted in #12742's description and in AICOMRCCL-2749 is wrong on this point; both are corrected.Issue Tracking
JIRA ID: AICOMRCCL-2749
Found while verifying the CPX work in #12742, and listed there as follow-up 5. Independent of #12742 and #12841: it predates them and neither touches this guard. The 8-rank run in #12742 needed
NCCL_MULTI_RANK_GPU_ENABLE=1to get off the ground; with this change it does not.Test Plan
Host tests (no GPU), three new cases on the existing
InitMicrotestrung:SameGpuUuidDifferentPciFunctions_IsAllowed— the CPX shape; the regression test for this defect.SameGpuUuidSamePciFunction_StillRefused— a real duplicate is still refused, so the fix is not the workaround in disguise.SamePciFunctionDifferentGpuUuids_IsAllowed— the MLOPart shape stays admitted.PeerSpecgains abusIdfield; it defaults to 0, which is what the fakedgetBusIdgives the self slot, so the three pre-existing guard tests are unaffected.Hardware, MI300X CPX (8 partitions per OAM, NPS1) on
dell300x-ccs-aus-k13-09, ROCm 7.0.1 host. A and B differ by this one line only — B was relinked from the same build tree after deleting the conjunct.Test Result
SymKernelMaskTestTMA skips).SameGpuUuidDifferentPciFunctions_IsAllowedfails and the other two still pass.NCCL_MULTI_RANK_GPU_ENABLE):ncclInvalidUsage,init.ccWARN on every rank — the defect reproduces.Out of bounds values : 0 OKat every size up to 64 MB, avg bus bandwidth 34.8 GB/s, exit 0.NCCL_TESTS_DEVICE=0): still refused, sameinit.ccWARN — the guard keeps its value.c5414a56413bd889; KFD node 2'sunique_idis14213723633280080009=0xC5414A56413BD889.