Repository navigation
Conversation
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: none of the three behaviour changes has a test, though anvil_engine_map.hpp already holds the pure-helper seam that would give them one.
Comment @argus review to re-run, or @argus re-review after pushing.
There was a problem hiding this comment.
ARGUS
Findings
3 finding(s) that could not be posted as inline comments
projects/rocshmem/tests/unit_tests/anvil_engine_map_gtest.cpp:53(Low) usableSdmaEngines' round-up is never exercised because every total the suite passes is even Every total this suite passes tousableSdmaEnginesis even (16, 8, 4, 2, 0), so the+ 1round-up at anvil_engine_map.hpp:43 is never exercised and a plainnumSdmaEnginesTotal / 2would keep the file green. That round-up is what guaranteesusableSdmaEnginesis at least 1 wheneveroamMapEngineNeedsFoldreturns true, which is what keepsfoldOamMapEngine's modulo off zero, so it is worth one assertion of its own.projects/rocshmem/src/sdma/anvil.cpp:101(Low) describeSelection still derives preferredEngine with __builtin_ctz, so the log names a different engine than the one selecteddescribeSelectionin projects/rocshmem/src/sdma/anvil.cpp, line 101, still derivespreferredEnginewith__builtin_ctz(s.preferredMask), butgetSdmaEngineIdat anvil.cpp:716 now selects withpreferredUsableEngine. The two disagree whenever the mask's lowest set bit is odd: for mask 0x6 the log printspreferredEngine=1while the queue is pinned to engine 2. Both consumers of this string are failure logs (the engine-pinned create error at anvil.cpp:230 and the missing xGMI physical id error at anvil.cpp:653), so the one diagnostic that explains an engine choice names an engine that was never used. CallingpreferredUsableEngine(s.preferredMask)at line 101 would keep the log and the selection on the same helper.projects/rocshmem/tests/unit_tests/anvil_engine_map_gtest.cpp:151(Low) Three retained budget cases still pass a raw engine total into the parameter this PR redefined as the usable even-engine count This PR redefinedqueueBudgetExceeded's third parameter fromnumEnginesTotalto the usable (even) engine count (anvil_engine_map.hpp:97-100) and updatedQueueBudgetRefusesOverSubscriptionto a literal 2, but three cases still passkCpxTotalthere: anvil_engine_map_gtest.cpp lines 151, 155 and 156. The assertions do not change, since 2 is a valid value under either reading, so the file simply documents two meanings for one parameter. PassingusableSdmaEngines(kCpxTotal)or a literal 1 at those three lines would make them say what the parameter now means.
Bottom line: Looks approval ready. The one open point is from the previous round: the parity fix at anvil.cpp:716 has no test behind it, so reverting preferredUsableEngine to __builtin_ctz keeps the suite green.
Comment @argus review to re-run, or @argus re-review after pushing.
d51b185 to
62768b7
Compare
fe53038 to
a674a04
Compare
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>
…o two OAMs (#12841) This PR is the rocSHMEM half of the CPX GIN fix. It also adds `anvil_engine_map.hpp` to RCCL's `--rocshmem-gin` copy list, so that header lands with `anvil.hpp`. **#12742 depends on this one** and must merge after it: `gin_anvil_sdma_factory.cc` calls `disconnectDevice()` and its test calls `oamMapEngineNeedsFold()` and `pciFunctionBus()`, all added here. Those call sites sit inside `#ifdef ENABLE_ROCSHMEM_GIN`, so default builds of #12742 are unaffected in the meantime. The engine `foldOamMapEngine()` picks is pinned only by this PR's `anvil_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. 1. **`getOamId` read a sysfs node that does not exist for a partition.** A CPX partition is a HIP alias at PCI function `.1`–`.7` with no sysfs node of its own, so the `xgmi_physical_id` 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. 2. **Engine selection used an SPX-only engine map.** `mi300xOamMap` indexes 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 onto an even engine in the reported range, and report failure through `SdmaQueue::valid()` so `connect()` fails and the caller can drop the backend instead of aborting. A rejected engine-pinned queue is not retried as a generic `HSA_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_MEMORY` part way through. In RCCL, an over-subscribed mesh fails `ncclCommInitRank` rather than degrading to a non-GIN path; that behaviour is pre-existing for any `connect()` 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 take `disconnect()`'s whole-process path. `sdmaHostInit`'s failure path and `sdmaHostStop` both use it, so no rocSHMEM path clears the process-global map any more. **rocSHMEM init fails fast on an SDMA connect failure.** `sdmaEnabled` also 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 in `anvil_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](https://amd-hub.atlassian.net/browse/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: - an HSA preferred mask uses its lowest even engine, or the even engine below an odd-only mask - the OAM-map fold spreads peers across the even engines the node reports; CPX has only engine 0 - `connect()` refuses an odd id - a rejected pinned create fails instead of retrying as a generic queue That 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](https://amd-hub.atlassian.net/browse/ROCM-32719)), documented in #13237, and not caused by the engine selection here. ## Test plan - [x] `AnvilEngineMap.*`: 19/19 pass on `banff-cyxtera-cx56-5` in SPX, DPX, QPX, and CPX, at `b47b4f7ccf`. - [x] Functional alltoall (`-a 19 -w 1`) with `ROCSHMEM_SDMA_ENABLED=1` on 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. - [x] Over-budget and rejected pinned creates abort init instead of hanging. CPX at 2+ channels and QPX at 32 ranks hit the even-engine queue budget. - [x] `rccl-UnitTestsGinAnvilSdmaFactory` (the test binary in #12742) on a CPX node, run on the combined change before the split: **12/12 pass, none skipped** (17/17 including the `AnvilOamMapEngine` cases). Run on `dell300x-ccs-aus-k13-09` (8x MI300X) switched to CPX. `Create_Success_1Rank` asserts `tryCreate(...) == 0`, which is only reachable once `createStatus() == HSAKMT_STATUS_SUCCESS` and `valid()`, so an SDMA queue really was created on a partition. - [x] `install.sh -l --rocshmem-gin` builds clean (0 errors) against ROCm 7.13. Review history for the combined change lives on #12603 (closed) and #12742. JIRA ID: AICOMRCCL-2387 [ROCM-32598]: https://amd-hub.atlassian.net/browse/ROCM-32598?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [ROCM-32719]: https://amd-hub.atlassian.net/browse/ROCM-32719?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- 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>
62768b7 to
20031ec
Compare
a674a04 to
92bc7a3
Compare
|
Thanks for the three findings in the
|
|
@abouteiller, this is stacked on #13235, so please review that one first; this PR adds a single commit on top of it. All Argus threads are addressed. It replaces #12841's even-engine-only restriction with a slot-level rule: only doorbell slots 0-3 of odd SDMA engines are unreachable from GPU-written doorbells (ROCM-32598), so odd engines carry traffic again. On MI300X it passed alltoall in SPX, DPX and CPX against the even-only build; CPX bandwidth rose from 323 to 369 GB/s, and SPX is unchanged. The results were taken at The red Thank you! |
|
Converting to draft while the driver team looks at ROCM-32598. #12841 has merged with the even-engine restriction, so the CPX/DPX alltoall and put hangs are already fixed on On Oct 22 the driver team (ROCM-32598) said they will investigate and update within a week. If they commit to a 32-byte-aligned SDMA doorbell layout, I'll close this and keep even-only, which stays correct after that change. If not, this is what's left before I mark it ready:
The red @abouteiller no need to review this one yet. #13235 is the one that's ready. |
20031ec to
02d7854
Compare
92bc7a3 to
b288960
Compare
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>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Odd SDMA engines are not broken. A doorbell written from a GPU kernel reaches the SDMA instance that owns the start of its 32-byte block, and amdgpu gives each instance 0x50 bytes of doorbells, so the first 16 bytes of every odd instance's range share a block with the previous instance. On MI300X, queue slots 0-3 of every odd engine never see a GPU-written doorbell and slots 4-7 work, in every compute partition mode (ROCM-32598). Go back to the engine selection of the base (HSA preferred engine, OAM map, partition fold over all engines). After each engine-pinned create, check the doorbell KFD assigned; a queue on an unreachable slot is kept parked, without device handles, so the engine's next create gets a reachable slot. Parked queues are released with disconnect(). The queue budget counts the reachable slots of every engine: 96 on SPX, 48 on DPX, 24 on QPX and 12 on CPX, instead of 64/32/16/8 for even engines only. A rejected pinned queue still does not retry as a generic queue, since KFD chooses its slot. AICOMRCCL-2880 Co-authored-by: Cursor <cursoragent@cursor.com>
b288960 to
add560d
Compare
Resource usage report (4gfx1201) |
Resource usage report (8gfx942) |
JIRA: AICOMRCCL-2880 · driver defect: ROCM-32598
Based on
develop. #12841 and #13235 have both merged, and this branch is one commit on top of them. It relies on #13235's node-local agreement, which turns a connect failure into the IPC fallback on every rank.Problem
Anvil rings every SDMA doorbell from a GPU kernel, and some doorbell slots never see a write made that way. Host-written doorbells always work, so ROCr's own SDMA queues are unaffected. Before #12841, rocSHMEM alltoall and put hung as soon as a transfer took the SDMA path on CPX (MI300X and MI355X) and on DPX (MI300X), including with the default
ROCSHMEM_SDMA_NUM_CHANNELS=1. #12841 avoids the problem by using even engines only. This PR replaces that restriction with the slot-level rule below, so odd engines carry traffic again.A multi-queue probe (attached to ROCM-32598) on MI300X SPX, covering all 16 engines x 8 slots, shows the pattern:
A GPU-written doorbell reaches the SDMA instance that owns the start of its 32-byte block, while a host write reaches the instance that owns its exact address. amdgpu gives each instance 0x50 bytes of doorbells (
sdma_doorbell_range = 20dwords), which isn't a multiple of 32, so the first 16 bytes of every odd instance's range share a block with the previous instance. KFD puts slots 0-3 of each engine there (slot q atbase + (q & 1) * 0x1000 + (q >> 1) * 8). All 128 slots match this rule, and DPX, QPX and CPX use the same layout. The driver-side fix (a 32-byte-aligned doorbell layout) is tracked in ROCM-32598. This PR is the rocSHMEM workaround until then.Summary
gpuDoorbellMisrouted()(pure, inanvil_engine_map.hpp) decides from a queue's doorbell address whether a GPU write reaches it.SdmaQueuechecks its doorbell. A queue on an unreachable slot is kept parked: it holds the slot but gets no device handles. KFD hands out an engine's first free slot, so the next create on that engine lands on a reachable one. Parked queues are released bydisconnect()and at teardown.isUsableSdmaEngine,usableSdmaEngines,preferredUsableEngine, and the odd-id refusal inconnect()). Engine selection uses the HSA preferred engine, the OAM map, and the partition fold over all engines. Odd engines are used again, but only through slots 4-7.sdmaQueueBudget): 96 on SPX, 48 on DPX, 24 on QPX and 12 on CPX, instead of fix(rocshmem): make Anvil SDMA usable on MI300X CPX and QPX, DPX up to two OAMs #12841's 64/32/16/8 for even engines only.HSA_QUEUE_SDMAin any mode, because KFD would pick that queue's slot. The connect fails, and fix(rocshmem): agree on the SDMA fallback across node-local ranks #13235 turns that into the agreed IPC fallback.gpuDoorbellMisroutedagainst all 128 measured MI300X slots, page-offset handling, reachable slots per engine, and the budget per mode.Cost: an odd engine gives up 4 of its 8 slots while this is in place, and those slots stay held by the process. SPX is unaffected, because its HSA-preferred engines are all even.
Known issue: intermittent 16-rank DPX hang with two workgroups per queue
Unrelated to slot choice, and also seen with the even-only revision: in full-node 16-rank DPX alltoall with
-w 2, about 1 run in 13 strands one queue withRB_ENABLE = 0andCONTEXT_STATUS = 0x44on a reachable slot. A standalone probe reproduces it in SPX without rocSHMEM when a second doorbell arrives while a 256 KiB copy is in flight. It's tracked as ROCM-32719.-w 1makes it much rarer, andROCSHMEM_SDMA_ENABLED=0avoids it.Separately,
SdmaImpl::sdmaDirtyis a 64-bit mask indexed bylocal_pe * numChannels + ch. A full DPX node (16 local PEs) overflows it at more than 4 channels, and a full QPX node (32) at more than 2. The budget or KFD refuses those configurations today.Test plan
Results were taken at
fe53038ed9, before the stack was rebased onto the current #12841 tip. The current headadd560d850(rebased ontodevelopatd6d2046bef, after #13235 merged; the patch is unchanged froma674a04501) has the same Anvil code apart from #12841's quieter partition-engine log (LOG_TRACEinstead ofLOG_INFO), and has not been rerun on hardware.MI300X
banff-cyxtera-cx56-5(8 GPUs, NPS1, gfx942, ROCm 7.2.0, amdgpu 6.16.13, Open MPI 4.1.6).USE_IPC=ON USE_SDMA=ON, functional test 19 (alltoall) with-w 1 -z 64 -s 1048576. Each mode was set withamd-smi set --compute-partition, and the node was left in SPX. "Even only" is an earlier even-engine revision of this stack (ea1c0c075e); #12841's own results atb47b4f7ccfare in its description. Bandwidth is the last row (8 MiB per rank pair).ea1c0c075e)ROCSHMEM_SDMA_FAIL_CONNECT_LOCAL_RANK=3,ROCSHMEM_SDMA_ENABLED=0, 2-rank testsEngine choice was checked with
BUILD_DEBUG_TRACE_HOST=ONbuilds, 8 ranks at 1 channel. No run logged a generic-queue retry or an abort.rocshmem_unit_tests --gtest_filter='AnvilEngineMap.*:SdmaAgreement.*': 26 passed in every mode (18AnvilEngineMap, 8SdmaAgreement)mpirun -np 2 rocshmem_unit_tests --gtest_filter='Sdma*'on MI300X SPX: 88 passed, no exclusions (same as even only)Sdma*suite stops with the samehipMallocout-of-memory abort after 110, 62 and 38 passing tests, identical to the even-only build and to the fix(rocshmem): agree on the SDMA fallback across node-local ranks #13235 headgpuDoorbellMisroutedAnvilOamMapEngine.*andAnvilPciFunctionBus.*cases from fix(rccl): GIN gate and cuMem window registration for MI300X CPX #12742 pass against this PR'sanvil_engine_map.hpp(standalone compile)-w 2: intermittent hang, ROCM-32719 (known issue above)Made with Cursor