Repository navigation
fix(rocshmem): make Anvil SDMA usable on MI300X CPX and QPX, DPX up to two OAMs - #12841
Conversation
Carried over from #12742
Low —
Low — the
Note that the second one spans both PRs: |
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Both carried-over findings implementedPushed as
The queue budget was only observable through a log line. That left No functional change on this PR — both are visibility only. Coverage (in
24/24 pass on an MI300X node (gfx942, ROCm 7.0.1). Note the merge order: this PR has to land before #12742, since the tests there reference |
|
| Workflow | Paths |
|---|---|
cuid-formatting.yml |
projects/cuid/** |
lstt-formatting.yml |
projects/rdc/**, projects/amdsmi/**, projects/rocm-smi-lib/** |
pre-formatting.yml |
only PRs targeting branch testbranch |
rocjitsu-formatting.yml, rocprofiler-*-formatting.yml, hip-formatting.yml, profiler-hub-formatting.yml |
their own projects |
Grepping all nine for projects/rccl or projects/rocshmem returns zero matches. So policy_check.py polls for a check run that is never created, and after its 900-second budget exits 1 with:
❌ Timed out waiting for required checks to complete.
{
"therock-pr-bot": "null",
}
The dump lists every check run that does exist on the SHA; pre-commit is absent from it, including on the original push-triggered run.
Why rerunning does not help. I tried gh run rerun --failed and a full-workflow rerun. Both reproduce the above verbatim. The documented remedy of an empty commit does not help either, since pre-commit still will not run.
Worth noting the policy content passes — the bot's own comment reports "✅ All Policy Checks Passed" (the one Unit Test item is a policy.yml:151 already warns about a related circular-dependency hazard in this same list.
Suggested fix (for whoever owns the bot): either extend a formatting workflow to cover projects/rccl/** and projects/rocshmem/** so pre-commit actually runs, or treat an absent required check as not-applicable rather than pending.
@ROCm/rccl-leads — flagging for the override.
|
@argus review |
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: connect() can still exit the process through getHipGpuAgent at anvil.cpp:531 when a HIP device has no mapped HSA agent, so the new fail-instead-of-abort path does not cover that case.
Comment @argus review to re-run, or @argus re-review after pushing.
|
Pushed
@argus re-review |
There was a problem hiding this comment.
ARGUS
Findings
2 finding(s) that could not be posted as inline comments
projects/rocshmem/src/sdma/anvil.cpp:195(Low) The constructor's alloc and map failures still abort, so the valid() contract does not cover the whole graceful path Atanvil.cpp:195and:197,hsaKmtAllocMemoryandhsaKmtMapMemoryToGPUstill go throughCHECK_HSAKMT_SUCCESS, which isLOG_ERROR_EXIT, so the constructor can still kill the process before reaching the graceful path at line 231 and thevalid()contract inanvil.hpp:131does not cover them. Both lines are outside this diff, so purely a question: is finishing that path in scope here, or a follow-up?projects/rocshmem/src/sdma_policy.cpp:37(Low) No in-repo build configuration sets USE_SDMA, so the new anvil code gets zero compile coverageUSE_SDMAdefaults OFF atprojects/rocshmem/CMakeLists.txt:50,src/CMakeLists.txt:110gates the sdma directory on it,CMakeLists.txt:337forces it back OFF when libnuma is missing, and none of the scripts underscripts/build_configsenables it, includingall_backends. So the new anvil code has no compile coverage in any configuration visible here. Is there a job outside this tree that builds withUSE_SDMA=ON?
Bottom line: Worth settling before approval: the partition || fold in oamMapEngineNeedsFold is the fix itself and nothing in this repo exercises it, though tests/unit_tests already has the if (USE_GDA) pattern to host one.
Comment @argus review to re-run, or @argus re-review after pushing.
|
Pushed
CI note: @argus re-review |
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: sdma_policy.cpp:78 takes disconnect()'s whole-process path on a failed connect, which anvil.hpp:114 says a failed create must not do.
Comment @argus review to re-run, or @argus re-review after pushing.
|
@dlamd1dai the current head ( |
|
Pushed two commits for the latest re-review:
@argus re-review |
|
Pushed @argus re-review |
## 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>
Anvil SDMA could not initialize on a CPX partition. Two defects: getOamId read xgmi_physical_id from a sysfs node that does not exist for a partition. A CPX partition is a HIP alias at PCI function .1-.7 with no node of its own, so the open failed. Fall back to the physical function-0 BDF, and fold the PCI function into the engine id so same-device peers do not all land on engine 0. Engine selection indexed mi300xOamMap, which describes the 14 xGMI SDMA engines of an unpartitioned MI300X. A CPX partition owns 2 host engines and 0 xGMI engines, so every map id was out of range and hsaKmtCreateQueueExt returned INVALID_PARAMETER. Fold the undoubled map value plus the PCI function into the reported range, retry with a generic HSA_QUEUE_SDMA when the engine-pinned queue is still rejected, and report failure through SdmaQueue::valid() so connect() fails instead of aborting. KFD caps user SDMA queues per engine, so a partition also has a small total budget. Count usage across both engines and refuse a request that cannot fit before KFD returns NO_MEMORY part way through building it. Adds disconnectDevice() so a caller can release the queues it took for one destination without tearing down the process-global channel map. Verified on dell300x-ccs-aus-k13-09 (8x MI300X, CPX): SDMA queues create on a partition and both partition engines are used for same-device peers. JIRA ID: AICOMRCCL-2387 Co-authored-by: Cursor <cursoragent@cursor.com>
Both helpers decided behaviour that no test could reach. pciFunctionBus was a file-static in anvil.cpp. It is pure and it picks the same fallback as oamMapEngineNeedsFold and foldOamMapEngine -- a tail outside '0'-'7' leaves function at -1, which collapses getOamId's candidate list and skips the physical-BDF read -- so it belongs beside them in the header as inline rather than hidden in the translation unit. The per-process queue count was only observable through a log line, which left connect()'s refusal and disconnectDevice()'s teardown with nothing to assert against: a refused connect has to leave the count unchanged and a fully abandoned create has to return it to where it started. queuesUsed() makes that readable without widening anything else. No functional change. Co-authored-by: Cursor <cursoragent@cursor.com>
The rocshmem precheckin Docker stage failed on hyd-7c-zt05-02 with 'auth.docker.io/token: context deadline exceeded' 1m47s into a 5m timeout. No code involved; the other node built rocSHMEM to 94% on the same commit. Co-authored-by: Cursor <cursoragent@cursor.com>
- connect() refuses a source device with no mapped HSA agent instead of reaching getHipGpuAgent's process exit through createSdmaQueue. - The queue-create error logs the generic retry's own status. - The post-create budget hint no longer depends on a known budget. - sdmaHostInit still allocates the (all-null) handle array after a failed connect, since the USE_SDMA testers index it unguarded. - isSdmaPartition() is the single copy of the partition predicate. - Align oamMapEngineNeedsFold's continuation line. Co-authored-by: Cursor <cursoragent@cursor.com>
- Move the pure engine-selection helpers into anvil_engine_map.hpp, which has no HSA/KFD dependency, and cover them with an unconditional rocSHMEM gtest so CI exercises the partition fold without USE_SDMA. - A failed SDMA engine-count query now reports all counts as unknown (total 0) at ERROR level instead of reading as a CPX partition. - createSdmaQueue charges the queue budget itself, so direct callers stay consistent with disconnectDevice. - connect() refuses an out-of-range destination device before getBusId can exit the process. Co-authored-by: Cursor <cursoragent@cursor.com>
sdmaEnabled also selects the alltoall algorithm, so clearing it on one rank while peers keep SDMA leads to incompatible synchronization and a hang. Abort instead; a cross-rank agreed fallback is tracked as AICOMRCCL-2795. Co-authored-by: Cursor <cursoragent@cursor.com>
0ac0547 to
f1742de
Compare
## 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>
|
RCCL RCCL builds the Anvil SDMA host code by copying rocSHMEM sources into its own build tree. In foreach(_f anvil.cpp anvil.hpp)This PR adds Seen on
foreach(_f anvil.cpp anvil.hpp anvil_engine_map.hpp)
|
abouteiller
left a comment
There was a problem hiding this comment.
Minor nits on comments and log level, ready for merge after that
|
There may be a build issue with rccl dues to new file engine_anvil_map.hpp being required as a dependent in the rccl import recipe. rccl side adaptations would need to land at the same time. |
The partition-engine line is diagnostic detail, and the missing-xGMI comment named a local machine. RCCL's --rocshmem-gin recipe copies a fixed source list, so anvil.hpp's new header has to be on that list or the copied translation unit cannot compile. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pushed aad8f6b.
|
Co-authored-by: Cursor <cursoragent@cursor.com>
foldOamMapEngine's engine choice is rocSHMEM's contract and changes with the ROCM-32598 workaround: #12841 folds onto even engines only, and #13237 folds over every engine again. The RCCL tests now assert only when the fold runs, and leave the engine it picks to anvil_engine_map_gtest. AICOMRCCL-2387 Co-authored-by: Cursor <cursoragent@cursor.com>
foldOamMapEngine's engine choice is rocSHMEM's contract and changes with the ROCM-32598 workaround: #12841 folds onto even engines only, and #13237 folds over every engine again. The RCCL tests now assert only when the fold runs, and leave the engine it picks to anvil_engine_map_gtest. AICOMRCCL-2387 Co-authored-by: Cursor <cursoragent@cursor.com>
) 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>
TheRock Submodule Bump ActivityNewest first
|
This PR is rebased onto `develop` (`e7cb4b477a`, head `126528dfdf`; only the CHANGELOG context changed since the tested head `7237ae7b96`) after #12751, and the check lives in that PR's `ginAnvilRegisterSignalSpan`. **Merge #14216 first:** with `NCCL_GIN_TYPE` unset on a multi-node communicator, Anvil can still be the selected backend until #14216 drops it there, and this check would then fail `ncclDevCommCreate` for Device API users instead of the communicator falling back to the GIN proxy. #12841 and #12742 from the CPX stack have merged; this PR does not depend on #13235 or #13237 and can merge in any order with them. The defect was found while fixing AICOMRCCL-2387 and is follow-up 4 on #12742. The CPX hardware runs below used the CPX stack so that Anvil could initialize on partitions. ## Motivation On MI300X CPX with 8 ranks and `NCCL_LSA_TEAM_SIZE=4`, the Anvil SDMA GIN backend (`NCCL_GIN_TYPE=7`) publishes signal addresses that are not mapped on the device. At init it only warns: ``` signal nRanks=8 != devr->lsaSize=4 ctx->rank=4 != devr->lsaSelf=0 ``` It then skips the connectivity check and keeps going. The signal binding path (`ginAnvilBuildSignalPeerAddrs`, called from `ginAnvilRegisterSignalSpan` and `ginAnvilBindContextSignals`) builds the peer signal table as `lsaSelf + (pe - ginRank) * bigSize` for every GIN peer. The device indexes this table by GIN peer (`remoteSignalAddr`, `resolveRemotePeerVa`), but each entry is a slot in this rank's LSA window. That is only correct for peers in the same LSA team, where GIN position and LSA slot agree. On rank 0, entries for GIN peers 4–7 point past the end of the window. On rank 4, entries for peers 0–3 point before its start. The ticket suggests rebasing the table on the LSA origin and extent. That would move the in-team peers' entries off their real slots, so this PR does not do that. Writing 0 for out-of-team peers is not safe either: `signalPeer` returns silently on a null address, so the wait would hang, and `ginAnvilResolvePeerVa` turns a null base into `0 + offset`. The same problem affects any `NCCL_GIN_CONNECTION_RAIL` GIN team with more than one rank. Its peers are on other hosts and never have a slot in the local window. This PR makes these configurations fail at registration with a clear error. The symmetric kernels, which request GIN internally and whose GIN peers are always rail peers outside the LSA team, now skip GIN on Anvil, so applications that never ask for GIN are not affected. ## Technical Details - `src/gin/gin_plugin_anvil_sdma.cc`: - New helper `ginAnvilFirstSignalPeerOutsideLsa(comm, nRanks, ginRank)`. For each GIN peer, it computes the LSA slot that the table entry points at. It accepts the peer only when that slot is inside `[0, lsaSize)` and `lsaRankList[slot]` is the peer's world rank. The GIN-to-world mapping uses the same stride as `teamRankToGinRank`: `contiguousRanksPerHost` under RAIL, otherwise 1. - `ginAnvilRegisterSignalSpan` calls the helper before it builds the peer table or registers the span in the IPC table. It runs once per resource-window bind, so it covers every logical context of every pending GIN context. If a peer fails, it logs a `WARN` naming the peer, GIN rank and size, `lsaSelf` and `lsaSize`. The warning tells the user to set `NCCL_LSA_TEAM_SIZE` to cover the GIN team or select another `NCCL_GIN_TYPE`. Registration then returns `ncclInvalidUsage`, which `ncclGinAnvilBindResourceWindowSignals` propagates to the caller that created the device communicator (see the next list). - The two old warnings are removed. Communicators whose GIN team matches their LSA team take the same path as before. - `test/gin/GinAnvilPlugin_test.cpp`: - New helper `expectSignalsUnbound`. It checks that a failed registration left `signals` and `signal_remote_addrs` null in every logical context of the handle, added nothing to the IPC table, and ran no connectivity check. - `ConnCheck_SkippedWhenLsaSizeMismatch` is now `BindSignals_GinPeerPastLsaWindowFailsRegistration`. - New test `BindSignals_GinPeerBeforeLsaWindowFailsRegistration`: the CPX case seen from the second LSA team (rank 2 of 4, `lsaSize` 2). - `ConnCheck_SkippedOnRailConnection` is now `BindSignals_RailGinTeamFailsRegistration`. In its layout, the rail peer's slot is inside the window but holds the wrong world rank, so the test only passes if the RAIL stride is applied. - `ConnCheck_UsesLsaTeamForBootstrap` used a reversed `lsaRanks` list that no real communicator produces. It now uses rank 1 with `lsaSelf` 1 and checks that the bootstrap uses the intra-node rank and peer list. - `connectColl` now takes the GIN rank. - `BindSignals_SpanBaseProbeSizedForWorldRanks` (added by #12751) used a two-rank RAIL GIN team, which now fails registration. It now uses one host of 8 ranks, where the rail team is this rank alone, so the world (8 ranks) is still wider than the GIN team (1 rank) and the probe buffer sizing is still tested. - `src/sym_kernels.cc`, `src/include/sym_kernels.h`: - New `ncclSymkState::ginBackendUnsupported`. `ncclSymkInitOnce` sets it when the symmetric kernels need GIN (LSA team smaller than the communicator) and `ncclGetRailedGinType` reports `NCCL_GIN_TYPE_ANVIL_SDMA`. In that case it logs an `INFO` line and does not add the GIN requirement to its internal device communicator, so Anvil signals are never bound during init. - `ncclSymkMask` treats GIN as unavailable when the flag is set. Because the mask keeps only GIN kernels when GIN is needed, this disables the symmetric kernels for that communicator, the same as `NCCL_SYM_GIN_KERNELS_ENABLE=0`; collectives use the regular kernels. - `ncclGinSetDefaultBackend` runs before `ncclSymkInitOnce` in `init.cc`, so the backend is known when the flag is computed. - `test/host/sym-kernels-test.cc`: replaces `ncclGetRailedGinType` with a hook. `SymKernelMicrotest.GinBackendUnsupported_OnlyForAnvilSdma` checks that only Anvil sets the flag (Anvil, proxy, none). `SymKernelMaskTest.NeedGinTrue_GinBackendUnsupported_EverythingCleared` checks that the mask is empty when GIN is needed and the flag is set. - `CHANGELOG.md`: added a "Resolved issues" entry. It notes that `ncclDevCommCreate` calls requesting GIN in these configurations used to succeed and now fail, and that the symmetric kernels no longer use GIN with Anvil, so communicator init and collectives are unaffected. Behavior changes reviewers should know about: - **Narrowed LSA team, including CPX across OAMs:** with `NCCL_GIN_TYPE=7` and `NCCL_LSA_TEAM_SIZE` smaller than the communicator, `ncclDevCommCreate` calls that request GIN used to succeed and now fail with `ncclInvalidUsage` and the new warning. `ncclCommInitRank` and collectives are unaffected: the symmetric kernels skip GIN on Anvil (see `sym_kernels.cc` above), so for these communicators they are disabled instead of failing init. On hardware, a plain `all_gather_perf` in this setup passes on this PR (see Test Result). - **RAIL with more than one GIN rank:** Device API GIN requests always fail on Anvil now. The symmetric kernels never use Anvil for it. - **Multi-backend runs:** with `NCCL_GIN_TYPE` unset, Anvil can be one of several active backends and ranks ahead of the GIN proxy. `ncclGinDevCommSetup` falls back between backends, but signal binding happens after that choice and has no fallback. #14216 drops Anvil on multi-node communicators so the proxy is chosen there; merge it first. On a single node with a narrowed LSA team, a Device API communicator that selected Anvil now fails instead of signaling through unmapped addresses. This has not been run multi-node. - **Connectivity check:** a communicator that passes registration reaches the skip branches for "GIN team size differs from LSA team size" and for RAIL only when its GIN team is smaller than its LSA team, or is a single-rank rail team. They are left in place. Related: - #14216 (merge first) drops Anvil when `comm->topo->nHosts > 1`. - #12751 (merged) rewrote this function as `ginAnvilBuildSignalPeerAddrs` / `ginAnvilRegisterSignalSpan`. The rebase carries the check into `ginAnvilRegisterSignalSpan` and removes the old warnings there; the second commit adapts the tests to per-context signal spans. The branch is now three commits; an attempt to refuse out-of-team peers in `createContext`, and its revert, were dropped. - The buffer path's IPC-table fallback in `regMrSym` (`RegisterVmm` with `lsaSelf` / `lsaSize`) has the same index-space mismatch. The stride path in `resolveRemotePeerVa` usually bypasses it, and this PR does not change it. ## Issue Tracking JIRA ID: AICOMRCCL-2706 ## Test Plan - `rccl-UnitTestsGinAnvilPlugin --gtest_filter='*BindSignals*:*ConnCheck*'`, then the full `rccl-UnitTestsGinAnvilPlugin` suite. Requires `ENABLE_ROCSHMEM_GIN` and ROCm 6.4 or later. - `rccl-UnitTestsMicroSymKernels` (host microtests). - rccl-tests `alltoall_perf` and `all_gather_perf` with `NCCL_GIN_TYPE=7`, on CPX ranks split across two OAMs (MI355X and MI300X) and on SPX (MI300X), each on this PR's library and on a baseline library with the plugin file from `develop`: - LSA team narrowed below the communicator: expect init to fail with the new warning instead of the old "nRanks != lsaSize" warnings. - LSA team unset: expect init and the connectivity check to pass, and `-D 3` runs to complete with the data check passing. - Plain collectives (no `-D`) with a narrowed LSA team: expect them to pass, with the symmetric-kernel skip logged on every rank. ## Test Result On MI355X with Linux 6.8, Anvil passes `alltoall_perf -D 3` and `all_gather_perf -D 3` data checks when the GIN and LSA teams match, in SPX with 8 ranks and in CPX with 4. When the LSA team is narrowed, this PR fails `ncclDevCommCreate` on every rank with the new warning, and plain collectives pass. On the same layout, the baseline skips the connectivity check and then hangs or fails. The MI300X host repeats the failure-path results. ### With the symmetric-kernel skip (`develop` `daf7ef33e4`, head `7237ae7b96`) Same MI355X host and build as the next section, SPX, 8 ranks, this PR's library only. Logs are under `~/pr13429/logs-port2`. - Full `rccl-UnitTestsGinAnvilPlugin`: 48 of 48 passed. - `rccl-UnitTestsMicroSymKernels`: 69 of 69 passed, including the two new tests. | Run | GIN team vs. LSA team | Result | Symmetric GIN skip logged | |---|---|---|---| | `alltoall_perf -D 3`, `NCCL_LSA_TEAM_SIZE=8` | 8 vs. 8 | Signals bound, conn-check OK on all 8 ranks, 0 wrong values | 0 ranks | | `all_gather_perf -D 3`, `NCCL_LSA_TEAM_SIZE=8` | 8 vs. 8 | Signals bound, conn-check OK on all 8 ranks, 0 wrong values | 0 ranks | | `alltoall_perf -D 3`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Init passes; the `ncclDevCommCreate` group fails (`common.cu:2705`, `invalid usage`) with the new warning on all 8 ranks | 8 ranks | | `all_gather_perf -D 3`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | rccl-tests refuses the kernel before creating a device communicator ("GinHybridAllGatherKernel requires CUDA P2P connectivity across all ranks"), rc 6; the baseline library prints the same refusal | 8 ranks | | Plain `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | **Passes**, 0 wrong values (failed `ncclCommInitRank` at the previous head) | 8 ranks | | Plain `all_gather_perf -R 2`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Passes, 0 wrong values | 8 ranks | | Plain `alltoall_perf -R 2`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Passes, 0 wrong values | 8 ranks | Example lines: ``` NCCL INFO Symmetric GIN kernels disabled: the Anvil SDMA GIN backend cannot reach rail peers outside the LSA team (lsaSize=4, nRanks=8) NCCL WARN GIN anvil-sdma: GIN peer 4 is outside the LSA team of rank 2 (ginRank=2, ginRanks=8, lsaSelf=2, lsaSize=4). Anvil SDMA can only signal LSA team peers; set NCCL_LSA_TEAM_SIZE to cover the GIN team or select another NCCL_GIN_TYPE. ``` The sections below are from earlier heads, before the symmetric-kernel skip. Where they say "Fails in `ncclCommInitRank`", this head passes instead. ### After the rebase onto `develop` (`1ba7048298`, head `2cb35ac234`) Built on the MI355X host below (gfx950, ROCm 10.1.0, Linux 6.8.0-101) with `install.sh --rocshmem-gin`, then `-DBUILD_TESTS=ON -DGIN_ANVIL_UNIT_TESTS=ON`. The baseline library uses `gin_plugin_anvil_sdma.cc` from `develop` (`1ba7048298`), which includes #12751. Logs are under `~/pr13429/logs-port` on that host. - `rccl-UnitTestsGinAnvilPlugin --gtest_filter='*BindSignals*:*ConnCheck*'`: 30 of 30 passed. - Full `rccl-UnitTestsGinAnvilPlugin`: 48 of 48 passed. **SPX, 8 ranks, one per GPU:** | Run | GIN team vs. LSA team | This PR | Baseline | |---|---|---|---| | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=8` | 8 vs. 8 | Signals bound (4 logical contexts per rank), conn-check **OK** on all 8 ranks, 0 wrong values | Same as this PR | | `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=8` | 8 vs. 8 | Signals bound, conn-check OK on all 8 ranks, 0 wrong values | Same as this PR | | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Init fails with `invalid usage` on all 8 ranks, new warning | Old warnings and conn-check skipped on all ranks, then **hangs** (killed at 300 s) | | `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Init fails, new warning on all 8 ranks | Old warnings and conn-check skipped, then the test fails (rc 6) | | Plain `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Fails in `ncclCommInitRank`, new warning | Passes, 0 wrong values, old warnings printed | | Plain `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=4`, `NCCL_SYM_GIN_KERNELS_ENABLE=0` | 8 vs. 4 | Passes, 0 wrong values | Passes | | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=4`, `NCCL_SYM_GIN_KERNELS_ENABLE=0` | 8 vs. 4 | Fails in the `ncclDevCommCreate` group (`common.cu:2705`), new warning | (not run) | The CPX cases were not re-run after the rebase, because the host was in SPX and changing its partition mode would disturb other users. The tables below are from the pre-rebase head. ### Before the rebase **Unit tests pass** (MI300X host below, ROCm 7.2.0, built at `9e444f3ab5` with `install.sh --rocshmem-gin`, then `-DBUILD_TESTS=ON -DGIN_ANVIL_UNIT_TESTS=ON`): - `rccl-UnitTestsGinAnvilPlugin --gtest_filter='*BindSignals*:*ConnCheck*'`: 23 of 23 passed, including the three new `BindSignals_*FailsRegistration` tests. - Full `rccl-UnitTestsGinAnvilPlugin`: 37 of 37 passed. Each hardware case ran twice: on this PR's library, and on a baseline library built from the same tree with `gin_plugin_anvil_sdma.cc` from `develop` (`2a4afbe5c4`). Common settings were `NCCL_GIN_TYPE=7`, `NCCL_CUMEM_ENABLE=1` and `-R 2 -D 3`, unless a row says "plain" (no `-D`). Runs were killed at 300 s. ### MI355X: `smci355-ccs-aus-m03-17` 8× MI355X (gfx950), NPS1, Linux 6.8.0-101, amdgpu 7.1-dev, ROCm 10.1.0, built at `8a97123fff`. No kernel-gate bypass was needed. Logs are under `~/pr13429/logs` on that host. **SPX, 8 ranks, one per GPU** (this PR's tree only): | Run | GIN team vs. LSA team | This PR | Baseline | |---|---|---|---| | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=8` | 8 vs. 8 | Signals bound (`lsaSize=8`), conn-check **OK** on all 8 ranks, 0 wrong values | Same as this PR | | `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=8` | 8 vs. 8 | Signals bound, conn-check OK on all 8 ranks, 0 wrong values | Same as this PR | | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=4` | 8 vs. 4 | Init fails with `invalid usage` on all 8 ranks, new warning | (not run) | **CPX, 4 ranks** (this PR plus the CPX stack: #12841, #13235, #13237 and #12742 at their 2026-10-14 heads, when the even-engine change was still in #13237). Ranks 0–1 are on two partitions of one OAM and ranks 2–3 on two partitions of another (`HIP_VISIBLE_DEVICES=0,1,8,9`, `NCCL_MULTI_RANK_GPU_ENABLE=1`): | Run | GIN team vs. LSA team | This PR | Baseline | |---|---|---|---| | `alltoall_perf`, LSA team unset | 4 vs. 4 | Signals bound, conn-check **OK** on all 4 ranks, 0 wrong values | Same as this PR | | `all_gather_perf`, LSA team unset | 4 vs. 4 | Signals bound, conn-check OK on all 4 ranks, 0 wrong values | Same as this PR | | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=2` | 4 vs. 2 | Init fails with `invalid usage` on all 4 ranks, new warning | Old warnings and conn-check skipped on all ranks, then **hangs** (killed at 300 s) | | `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=2` | 4 vs. 2 | Init fails, new warning on all 4 ranks | Old warnings and conn-check skipped, then the test fails (rc 6) | | Plain `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=2` | 4 vs. 2 | Fails in `ncclCommInitRank`, new warning | Passes, 0 wrong values, old warnings printed | | Plain `all_gather_perf`, `NCCL_LSA_TEAM_SIZE=2`, `NCCL_SYM_GIN_KERNELS_ENABLE=0` | 4 vs. 2 | Passes, 0 wrong values | Passes | | `alltoall_perf`, `NCCL_LSA_TEAM_SIZE=2`, `NCCL_SYM_GIN_KERNELS_ENABLE=0` | 4 vs. 2 | Fails in the `ncclDevCommCreate` group (`common.cu:2705`), new warning | (not run) | Example warning (rank 2, on the second OAM): ``` GIN anvil-sdma: GIN peer 0 is outside the LSA team of rank 2 (ginRank=2, ginRanks=4, lsaSelf=0, lsaSize=2). Anvil SDMA can only signal LSA team peers; set NCCL_LSA_TEAM_SIZE to cover the GIN team or select another NCCL_GIN_TYPE. ``` The baseline prints the warnings from the ticket on the same run, for example `signal nRanks=4 != devr->lsaSize=2 (rank 0)` and `ctx->rank=2 != devr->lsaSelf=0 (signal peer indexing may be wrong)`. ### MI300X: `banff-cyxtera-cx56-5` 8× MI300X (gfx942), NPS1, Linux 5.15, amdgpu 6.16.13, ROCm 10.1. Logs are under `~/pr13429/logs-r10` on that host. On this host, RCCL turns cuMem off below Linux 6.8, and without cuMem rccl-tests skips every `-D 3` run. These runs therefore add an experiment-only patch that skips that kernel gate (`RCCL_EXP_SKIP_KERNEL_GATE=1`). On this kernel, cuMem peer mappings are broken, so the matched-team runs fail the connectivity check on both libraries. The failure path still shows, because the new check runs before any peer is signaled. `NCCL_RMA_DISABLE=1` was used whenever the LSA team was narrowed, because the RMA proxy's memory registration fails on this host before GIN is reached. | Run | GIN team vs. LSA team | This PR | Baseline | |---|---|---|---| | SPX, 8 ranks, `NCCL_LSA_TEAM_SIZE=4`, `alltoall_perf` | 8 vs. 4 | Init fails with `invalid usage` on all 8 ranks with the new warning | Old warnings and conn-check skipped on all ranks, then **hangs** (killed at 300 s) | | SPX, same, `all_gather_perf` | 8 vs. 4 | Init fails, new warning on all 8 ranks | Old warnings and conn-check skipped, then the test fails (rc 6) | | CPX, 6 ranks, 3 on each of two OAMs, `NCCL_LSA_TEAM_SIZE=3`, `alltoall_perf` | 6 vs. 3 | Init fails, new warning on all 6 ranks | Old warnings and conn-check skipped, then **hangs** (killed at 300 s) | | CPX, same 6 ranks, LSA team unset | 6 vs. 6 | Signals bound, conn-check runs and fails on this kernel's broken cuMem mapping | Same as this PR | | SPX, 8 ranks, LSA team unset | 8 vs. 8 | Signals bound, conn-check runs and fails on the broken mapping | Same as this PR | | SPX, 8 ranks, `NCCL_LSA_TEAM_SIZE=4`, plain `all_gather_perf` | 8 vs. 4 | Fails in `ncclCommInitRank` | Passes, 0 wrong values, old warnings printed | | Same, `NCCL_SYM_GIN_KERNELS_ENABLE=0` | 8 vs. 4 | Passes | Passes | | SPX, 8 ranks, `NCCL_LSA_TEAM_SIZE=4`, `alltoall_perf`, `NCCL_SYM_GIN_KERNELS_ENABLE=0` | 8 vs. 4 | Fails in the `ncclDevCommCreate` group | (not run) | | SPX, 8 ranks, `NCCL_LSA_TEAM_SIZE=4`, plain `all_gather_perf`, `NCCL_GIN_TYPE` unset | 8 vs. 4 | Init fails in the GIN proxy backend (`gin_host_proxy.cc:477`), before Anvil binds | Same failure | ### Notes - The ticket's exact layout (8 CPX ranks, 4 on each OAM) could not be run on either host. On the CPX stack, every rank runs out of SDMA queues while connecting to 7 peers (`SDMA queue budget exhausted: 7 queue(s) taken by this process + 1 requested, limit 8`; on MI355X the driver returns `NO_MEMORY` for the 8th queue). That happens in `connect`, before signal binding, on this PR and on the baseline alike. On MI355X it happens with `NCCL_LSA_TEAM_SIZE=8` too, whether the 8 partitions are on one OAM, split 4+4, or spread one per OAM. The 4-rank (MI355X) and 6-rank (MI300X) layouts stand in for it. The limit of 8 is the even-engine CPX budget, which #12841 now ships; #13237's slot-aware budget raises it to 12, and the 8-rank layout has not been retried with that budget. - The CPX stack needed one local build fix at the time: RCCL's copy list lacked `anvil_engine_map.hpp`. #12841 now includes it. - Not run: multi-node, and the RAIL connection type on hardware. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…3235) JIRA: [AICOMRCCL-2795](https://amd-hub.atlassian.net/browse/AICOMRCCL-2795) Follow-up to #12841 (merged as `c34c1b1f56`), which makes rocSHMEM fail fast on an SDMA connect failure and names this fallback as its follow-up. It uses `disconnectDevice()` and `queuesUsed()` from #12841. Rebased onto `develop` after #12841 merged, so the branch carries only this PR's three commits. #13237 is stacked on this PR. It never retries a rejected engine-pinned queue as a generic queue, and relies on this agreement to turn that connect failure into the IPC fallback on every rank. ## Summary - A failed `AnvilLib::connect()` in `SdmaImpl::sdmaHostInit` no longer aborts. Whether SDMA is used also selects the alltoall algorithm: `internal_alltoallmem_wg` and `alltoall_wg` branch on `constmem.ipc_sdma_threshold`, so a rank without SDMA syncs on `alltoall_pSync` while SDMA peers wait for an AMO add that never comes. So each rank's result is all-gathered over the node-local IPC group, and a failure on any rank drops SDMA on all of them. - `init_constant_memory()` derives `constmem.ipc_sdma_threshold` from the agreed `sdmaEnabled` instead of `ROCSHMEM_SDMA_ENABLED`, so after a fallback no device path calls `sdmaCopy` on the null handle table. `sdmaEnabled` defaults to `false` until `sdmaHostInit` agrees on it. - Both init paths: `IpcSdmaImpl::ipcHostInit(..., MPI_Comm)` gathers over the node-local `shmcomm`, and the `TcpBootstrap` path uses `groupAllGather` over the same IPC ranks. `IpcOnImpl::ipcHostInit` hands them out through new optional out-parameters, so existing callers are unchanged. - On fallback, each rank releases only the peers it wired with `disconnectDevice()` (not the process-global `disconnect()`), checks `queuesUsed()` against its pre-init value, and fills `deviceHandles_d` with nulls, since `USE_SDMA` testers index it without checking `sdmaEnabled`. The all-disabled path publishes the same null table. Fallback and `sdmaHostStop` share one release helper. - Every rank logs the fallback at ERROR and names the local rank(s) that failed. A `ROCSHMEM_SDMA_ENABLED` value that differs across ranks used to give the same hang; it now falls back the same way, with its own message. - An undocumented debug hook, `ROCSHMEM_SDMA_FAIL_CONNECT_LOCAL_RANK=<n>`, makes local rank `n` wire every peer normally and then report a failed connect in its own status before the all-gather, so the fallback and release path can be exercised on any partition mode. It lives in `sdma_agreement.hpp` (`applyForcedFailure()`) rather than in the connect loop. It is read with `getenv` and not registered in `envvar`, so it stays out of the docs, the help output and `rocshmem_info`. - The decision logic lives in the HSA-free header `sdma_agreement.hpp`, with an unconditional gtest (`SdmaAgreement.*`), so CI covers it without `USE_SDMA`. ## Test plan Run on the rebased head (`20031ec663`) on one MI300X node (`banff-cyxtera-cx56-5`, 8 GPUs, SPX, ROCm 7.2), built with `USE_IPC=ON USE_SDMA=ON GPU_TARGETS=gfx942`, Release. SDMA cases set `ROCSHMEM_SDMA_ENABLED=1`, since it defaults to 0. "Before" is the same tree without `20031ec663`, which still took the device threshold from the env. The branch was later rebased onto `develop` (`20031ec663` is now `02d78541f4`, patch unchanged), `b07efef44a` moved the debug hook out of `envvar`, and `7a815ef879` moved the forced failure from the connect loop to the agreement step. The "After" column was rerun on the same node on `7a815ef879`, with the same results; see [the hardware check comment](#13235 (comment)). | Case | Before | After | |---|---|---| | 2 ranks, put / get / put-nbi / get-nbi (16 KiB) and WG put (1 MiB), SDMA on | pass | pass | | Same, forced failure on local rank 1 | put, put-nbi, get-nbi, WG put: data validation error; get: abort (rc 134) | all pass, both ranks fall back, 0 queues held | | Same, `ROCSHMEM_SDMA_ENABLED=0` on rank 1 only | same failures as forced | all pass, both ranks fall back | | Same, SDMA off everywhere | pass | pass | | 8-rank alltoall (`-a 19 -w 1 -z 64 -s 1048576`), SDMA on, MPI and TcpBootstrap | pass (1.6-1.7 TB/s at 8 MiB) | pass (1.6-1.7 TB/s) | | Alltoall, forced failure on local rank 3 or 7 (MPI) or 5 (TcpBootstrap) | data validation error on every rank | pass, all 8 ranks fall back, 0 queues held, 56 GB/s | | Alltoall, `ROCSHMEM_SDMA_ENABLED=0` on ranks 4-7 only | hang (timeout) | pass, all 8 ranks fall back | | Alltoall, SDMA off everywhere | pass | pass | - [x] `rocshmem_unit_tests --gtest_filter='SdmaAgreement.*:AnvilEngineMap*'`: 27 passed - [x] On `7a815ef879`, `sdma_agreement_gtest.cpp` built standalone with `g++ -Werror` (no ROCm needed): 11 `SdmaAgreement` tests passed, including the new forced-failure tests - [x] On `7a815ef879`, on MI300X: `rocshmem_unit_tests --gtest_filter='SdmaAgreement.*'` 11 passed, and the `mpirun -np 2` SDMA unit tests 80 passed on both ranks - [x] `mpirun -np 2 rocshmem_unit_tests --gtest_filter='SdmaSimpleCoarse/*:SdmaSimpleFine/*:SdmaTiledFine/*'` with SDMA on: 80 passed on both ranks - [x] Earlier, before the rebase (`d51b1856f5`): the same fallback on MI355X SPX, and a real queue over-subscription on MI355X and MI300X CPX with no debug env var, where every rank fell back and completed alltoall while the #12841 head aborted. See the CPX comments below. Made with [Cursor](https://cursor.com) [AICOMRCCL-2795]: https://amd-hub.atlassian.net/browse/AICOMRCCL-2795?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This PR is the rocSHMEM half of the CPX GIN fix. It also adds
anvil_engine_map.hppto RCCL's--rocshmem-gincopy list, so that header lands withanvil.hpp.#12742 depends on this one and must merge after it:
gin_anvil_sdma_factory.cccallsdisconnectDevice()and its test callsoamMapEngineNeedsFold()andpciFunctionBus(), all added here. Those call sites sit inside#ifdef ENABLE_ROCSHMEM_GIN, so default builds of #12742 are unaffected in the meantime. The enginefoldOamMapEngine()picks is pinned only by this PR'sanvil_engine_map_gtest, not by RCCL's tests, because #13237 changes it.Summary
GIN Anvil SDMA (
NCCL_GIN_TYPE=7) could not initialize on MI300X in CPX mode. Two defects live in rocSHMEM.getOamIdread a sysfs node that does not exist for a partition. A CPX partition is a HIP alias at PCI function.1–.7with no sysfs node of its own, so thexgmi_physical_idopen failed. Fall back to the physical function-0 BDF, and fold the PCI function into the engine id so same-device peers do not all land on engine 0.Engine selection used an SPX-only engine map.
mi300xOamMapindexes the 14 xGMI SDMA engines of an unpartitioned MI300X; a CPX partition owns 2 host engines and 0 xGMI engines, so every map id was out of range andhsaKmtCreateQueueExtreturnedINVALID_PARAMETER. Fold the undoubled map value plus the PCI function onto an even engine in the reported range, and report failure throughSdmaQueue::valid()soconnect()fails and the caller can drop the backend instead of aborting. A rejected engine-pinned queue is not retried as a genericHSA_QUEUE_SDMA, because that retry cannot choose the engine or doorbell slot.Queue budget. KFD caps user SDMA queues per engine. This PR counts only the even engines it can use: 1 on CPX (8 queues), 2 on QPX (16), 4 on DPX (32), and 8 on SPX (64), before the queues ROCm already holds. #13237 raises this to the GPU-reachable slots of every engine (12 / 24 / 48 / 96). Usage is counted across those engines and an over-subscribed request is refused up front rather than letting KFD return
NO_MEMORYpart way through. In RCCL, an over-subscribed mesh failsncclCommInitRankrather than degrading to a non-GIN path; that behaviour is pre-existing for anyconnect()failure, and an RCCL-side graceful fallback is a follow-up listed on #12742.New API.
disconnectDevice(int dstDeviceId)releases the queues held for one destination and decrements the budget by only the valid queues it removed. The channel map is process-global and a second communicator reuses a previous create's queues, so a failed create must not takedisconnect()'s whole-process path.sdmaHostInit's failure path andsdmaHostStopboth use it, so no rocSHMEM path clears the process-global map any more.rocSHMEM init fails fast on an SDMA connect failure.
sdmaEnabledalso selects the alltoall algorithm, so turning it off on one rank while its peers keep SDMA leads to mismatched synchronization and a hang. A connect failure now aborts init instead. #13235 (AICOMRCCL-2795) replaces this with a fallback that every node-local rank agrees on.Testability. The pure engine-selection and budget helpers (
isSdmaPartition,oamMapEngineNeedsFold,foldOamMapEngine,pciFunctionBus,isUsableSdmaEngine,usableSdmaEngines,preferredUsableEngine,queueBudgetExceeded) live inanvil_engine_map.hpp, which has no HSA/KFD dependency, and are covered by an unconditional rocSHMEM gtest (anvil_engine_map_gtest.cpp).AnvilLib::queuesUsed()exposes the budget count for tests.Even engines here; slot-aware use of odd engines is #13237
ROCM-32598 is a doorbell-slot defect, not a blanket failure of every odd engine. In every MI300 compute-partition mode, GPU-written doorbells never reach slots 0–3 of an odd-numbered SDMA engine. Slots 4–7 of those engines work, and every slot of an even engine works. Anvil rings every doorbell from device code.
This PR cannot choose the slot KFD assigns, so it conservatively places every queue on an even engine:
connect()refuses an odd idThat closes the earlier DPX hang, where pairs spanning four or more OAMs folded onto engines 1, 5, and 7, and the CPX hang where peers folded onto engine 1 (AICOMRCCL-2880).
#13237 is the follow-up that recovers the usable odd-engine capacity. After each pinned create it checks the doorbell address, parks a queue whose slot a GPU write cannot reach, and lets the next create on that engine take a reachable slot. This PR does not depend on that change.
A full-node 16-rank DPX run can still hang intermittently with
-w 2. That is a separate ring exception on a reachable slot (ROCM-32719), documented in #13237, and not caused by the engine selection here.Test plan
AnvilEngineMap.*: 19/19 pass onbanff-cyxtera-cx56-5in SPX, DPX, QPX, and CPX, atb47b4f7ccf.-a 19 -w 1) withROCSHMEM_SDMA_ENABLED=1on that node. Traced queues used even engines only: SPX 0/2/4/6/8/10/12/14, DPX 0/2/4/6, QPX 0/2, CPX 0. SPX 8 ranks reached 1.70–1.74 TB/s; DPX 16 ranks at 1–2 channels reached 1.05–1.15 TB/s; CPX 8 ranks at 1 channel reached 320–324 GB/s. No generic-queue retry was logged.rccl-UnitTestsGinAnvilSdmaFactory(the test binary in fix(rccl): GIN gate and cuMem window registration for MI300X CPX #12742) on a CPX node, run on the combined change before the split: 12/12 pass, none skipped (17/17 including theAnvilOamMapEnginecases). Run ondell300x-ccs-aus-k13-09(8x MI300X) switched to CPX.Create_Success_1RankassertstryCreate(...) == 0, which is only reachable oncecreateStatus() == HSAKMT_STATUS_SUCCESSandvalid(), so an SDMA queue really was created on a partition.install.sh -l --rocshmem-ginbuilds clean (0 errors) against ROCm 7.13.Review history for the combined change lives on #12603 (closed) and #12742.
JIRA ID: AICOMRCCL-2387