[dlamd1dai]

Motivation

Anvil SDMA currently allocates one GPU context per connection and ignores nContexts. When ncclGinAllContexts barriers run on a shared connection, every logical context writes and waits on the same signal cells, so a later barrier can complete on a stale count from an earlier one.

Technical Details

Give each logical context its own ncclGinAnvilSdmaGPUContext and LSA signal stripe while keeping SDMA queues, dirty bits, and counters shared at the connection. Device APIs now index ctx.handle by ctx.contextId. Resource-window bind walks nContexts slots instead of one slot per plugin context.

Signal peer addresses are indexed in GIN-team space (ctx->rank), matching resolveRemotePeerVa; regMrSym keeps devr->lsaSelf because that table is LSA-indexed. The signal span is registered in the IPC table once per bind rather than once per context, and the comm-wide collectives (the local-base allgather and the LSA conn-check) run at span scope so a comm with N logical contexts still costs one bootstrap round trip, not N.

Two fixes to the surrounding code are folded in, previously proposed standalone as #12450 and #12451 and closed in favour of this PR once the restructuring above made them inapplicable as separate patches:

• The span base probe is sized with comm->nRanks and publishes the local VA at comm->rank. bootstrapAllGather writes one slot per world rank, so a GIN-team-sized buffer was overrun and the VA landed in the wrong slot under NCCL_GIN_CONNECTION_RAIL.
• The bind path snapshots g_pendingByComm into a vector under pluginMutex and walks the copy. Its three reads (span geometry, slot binding, and fail: rollback) were unlocked while ginAnvilPendingAdd/Remove/Clear all hold that non-recursive mutex; find() also avoids default-inserting an empty list for a comm with nothing pending.

JIRA ID: AICOMRCCL-2339

Test Plan

• Host plugin unit tests, including BindSignals_LogicalContextsUseDistinctSignalStripes and the ConnCheck_* suite
• Device template tests, including SignalApis_SelectLogicalContext and PutValue_SelectLogicalContext
• MPI regression Barrier_AllContextsConsecutiveSignalsDoNotAlias_SingleNode (Anvil, 1 node / 2 ranks, NCCL_GIN_TYPE=7, delayed second barrier)

Test Result

Anvil SDMA is NCCL_GIN_TYPE=7 (NCCL_NET_DEVICE_GIN_ANVIL_SDMA). Earlier runs in this section were recorded when the enum was 6.

Pre-rebase (commit 970a014315):

• rccl-UnitTestsGinAnvilPlugin: 20/20 passed on MI355
• GinAnvilSdmaTemplateTest.*: 13/13 passed on MI355 (before PutValue_SelectLogicalContext was added)
• GinMPIDeviceTests.Barrier_AllContextsConsecutiveSignalsDoNotAlias_SingleNode on MI355: PASSED (7779 ms) on smci355-ccs-aus-m03-17, 1 node / 2 ranks, NCCL_GIN_ENABLE=1, NCCL_CUMEM_ENABLE=1. Ran rccl-UnitTestsMPI inside rccl-gin-pr11556-rs:latest with --ipc=host. Host mpirun on the same node segfaulted in HIP IPC during communicator bring-up (ncclP2pImportShareableBuffer / ncclDevrCommCreateInternal) while other GPU docker jobs held /dev/kfd and /dev/dri; that path never reached the kernel.
• Same MPI filter on MI350: PASSED (8834 ms) on cv350-rck-g03-e23-08, 1 node / 2 ranks, same GIN env. Host mpirun -np 2; GPUs were idle. Used the gfx950 rccl-UnitTestsMPI/librccl.so.1 from the MI355 PR build (native CentOS 9 link of librccl.so failed on librocshmem_gin_host.a bitcode encoding; Docker was unavailable on that node).

Current head cf8cf9720f, rebased onto develop at 482e541856, retested from a clean worktree on MI355X (smci355-ccs-aus-m03-17), 1 node / 2 ranks, NCCL_GIN_TYPE=7, NCCL_GIN_ENABLE=1, NCCL_CUMEM_ENABLE=1, NCCL_GIN_NCONTEXTS=4:

• rccl-UnitTestsGinAnvilPlugin: 37/37 passed (304 ms), including the full ConnCheck_* suite and the two span-scope cases added for the ARGUS finding. Both were confirmed to fail against 8eafd3d752 (3 allgathers at nContexts=3) and pass here (1).
• Whole GinMPIDeviceTests.* suite on the Anvil backend: 44 passed, 20 skipped, 0 failed (40.1 s). Covers Barrier_AllContextsConsecutiveSignalsDoNotAlias_SingleNode (680 ms), all seven BarrierFence_*_SingleNode cases including AllContextsPut/AllContextsGet, MultiContext_AllFourRoute/NonPowerOf2/Exclusive, and Put_BasicAndOffsets_MultiContext/PutValue_Inline_MultiContext/WaitCounterAndSignal_MultiContext.
• The 20 skips are topology or opt-in gated, not regressions: cross-node and 4-rank cases, RailConnection_Create, the VASignal_* cases (VA signals are unsupported on SDMA by design), and the pool tests that require NCCL_GIN_SIGNAL_POOL_SIZE>=0x40000000.

An earlier attempt at this run skipped all 64 cases because NCCL_CUMEM_ENABLE was unset, which trips the Symmetric memory required gate; the numbers above are from the corrected invocation.

Earlier post-rebase run on f32754f6c1, same node and env, before #12450/#12451 were folded in: plugin 37/37, GinMPIDeviceTests.* 39 passed, 23 skipped, 0 failed (34.5 s).

Environment note: the host ROCm on that node is 7.0.2.1 while its KFD driver is newer, and hipIpcOpenMemHandle segfaults during plain P2P transport setup (ncclP2pImportShareableBuffer) before any GIN code runs. The runs above use the node's ROCm 7.13 runtime in a container with --ipc=host, where the same binaries pass. Not related to this change; the same workaround was needed for the pre-rebase runs.

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

Made with Cursor

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

[therock-pr-bot]

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

• ❌ PR Description

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

[amd-argus]

ARGUS

Bottom line: Worth settling before approval: each logical context now takes its own entry in the 16-slot Anvil IPC table, so a high ginContextCount fails ncclDevCommCreate.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

@argus re-review

Follow-up on the review of 99d2478 (now 757f9bab):

Thread	Status
IPC table: one entry per logical context	Fixed — span registered once via ginAnvilRegisterSignalSpan(); per-context bind only slices pointers
MPI regression not in gin_devmpi_sdma	Fixed — case added next to Barrier_TwoRanks; deterministic signal-stripe assertions added
SignalApis_SelectLogicalContext coverage gaps	Fixed — counter/signal APIs on contextId 0/1; new PutValue_SelectLogicalContext
Shared counters across contexts	Fixed — per-context counter slice (countersDev + contextId * nCounters), matching proxy/gdaki
Cycle-count oracle flakiness	Mitigated — post-barrier signal cell + pointer checks added
Plugin unit-test stride ambiguity	Fixed — distinct nSignals / nSignalsPerContext / nContexts in BindSignals_LogicalContextsUseDistinctSignalStripes
Duplicated teardown / signals = nullptr on error	Addressed in 91215b37

rocSHMEM scoped-atomics: split to #11591 so this PR stays scoped to AICOMRCCL-2339 signal de-aliasing. GIN CI on this PR may need that companion change (or an equivalent develop-side fix) to go green.

MPI on SUT: planning to run Barrier_AllContextsConsecutiveSignalsDoNotAlias_SingleNode on an Anvil 2-GPU node (NCCL_GIN_TYPE=6) and update the Test Result section once it completes.

[amd-argus]

ARGUS

Bottom line: Worth settling before approval: PutValue_SelectLogicalContext gives both contexts the same signal_remote_addrs array, so it passes with the fix reverted.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[amd-argus]

ARGUS

Bottom line: Worth settling before approval: the GIN backend gating at gin_plugin_anvil_sdma.cc:563 and the anvil-only build break in GinDeviceMPITests.cpp:1658.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

@argus re-review

Rebased onto current develop (7a722544d5) and addressed the open ARGUS threads:

Thread	Status
Empty pending list returns ncclInvalidArgument	Fixed — returns ncclSuccess after ginAnvilPendingClear so non-anvil backends (e.g. NCCL_GIN_TYPE=5) keep creating communicators
MPI GetSignalPtr build break without Anvil	Fixed — probe behind #if NCCL_GIN_ANVIL_SDMA_ENABLE; handle/contextId derived via ncclGin / ncclGinInitCommon
PutValue_SelectLogicalContext shared remote-addrs	Fixed — distinct signal_remote_addrs arrays per logical context
ResetSignal only on contextId=0	Fixed — moved under contextId=1; context 0 sentinel stays 11
Counter stride ambiguity (nCounters==nSignals)	Fixed — nCounters=5
Bind peer geometry on ctx->rank vs lsaSelf	Fixed — bind loop uses devr->lsaSelf, matching ginAnvilRegisterSignalSpan / regMrSym
Bind nContexts parameter naming	Fixed — renamed to nSignalSlots
Unused counters kernel arg	Fixed — dropped

Also kept develop's LSA signal conn-check on the first bind, and indexed Get/FlushAsync through anvilGpuCtx for the APIs that landed after this PR branched.

Follow-ups (separate PRs):

• GDA (NCCL_GIN_TYPE=5) still aliases ctx.handle without [contextId] — opening separately under AICOMRCCL-2339
• rccl-UnitTestsGinAnvilPlugin is already built by gin.sbatch but not executed — opening a CI wiring PR to run it

rocSHMEM: no longer in this diff (scoped-atomics landed via #11350). Removed the project: rocshmem label and rocshmem-reviewers request.

[dlamd1dai]

Follow-up PRs opened from current develop:

• GDA logical-context de-alias: fix(gin): de-alias rocSHMEM-GDA signals across logical contexts #12440
• Anvil plugin unit-test CI execution: ci(rccl): run Anvil plugin unit tests in GIN sbatch #12441

[amd-argus]

ARGUS

Bottom line: Worth settling before approval: hostAddrs at gin_plugin_anvil_sdma.cc:783 is sized in GIN-team ranks but anchored on devr->lsaSelf, so it reads past the allocation under RAIL.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

@argus re-review

Follow-up on 7a722544d5 (now 8eafd3d752):

Thread	Status
hostAddrs anchored on lsaSelf but indexed in GIN-team space	Fixed — span registration and per-context bind both use the GIN rank (ctx->rank). regMrSym still uses lsaSelf because that table is LSA-indexed.
lsaInputBase latched across SetLsaSelfAddr	Fixed — latch cleared in SetLsaSelfAddr
Stripe test never separates slot from contextId	Fixed — second pending context on the same comm; context 0 of the first context lands at slot 1

Follow-ups, not in this PR (based on develop; rebase onto this branch if it merges first):

• gatheredLocalBases sized to ctx->nRanks while bootstrapAllGather writes world nranks — fix(gin): size Anvil signal allgather to the world communicator #12450
• g_pendingByComm walked without pluginMutex — fix(gin): snapshot Anvil pending contexts under pluginMutex #12451

Retest plan stays the single-node Anvil barrier (NCCL_GIN_TYPE=7, 1 node / 2 ranks).

[amd-argus]

ARGUS

Bottom line: Worth settling before approval: the bind body's comm-wide allgather at gin_plugin_anvil_sdma.cc:798 now runs once per logical context instead of once per connection.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

CI status on 8eafd3d752 — two items need a maintainer

Everything that can run from a fork is green: mci/rocm-libraries/extra(rccl), mci/rocm-libraries/static-analysis(rccl), Host unit tests (CPU-only), Test rccl (shard 1/1) (gfx94X-dcgpu), the comm-libs and emulation build stages, and both TheRock summaries. The two red checks are infrastructure, not this change:

Check	Why	What unblocks it
RCCL PR gate	IS_FORK=true, so coco smoke is never auto-dispatched (GATE_RESULT=skipped). The job's own error text says a maintainer must dispatch it.	Please dispatch the RCCL coco PR workflow with this PR's pr_number and pr_sha, confirm it passes, then override the check.
therock-pr-bot	policy_check.py hit its 900 s limit while the emulation and Windows compiler-runtime stages were still null. The policy table itself is all-pass.	Re-run once the slower build stages finish, or override.

Separately, mci/rocm-libraries/precheckin(rccl) build 6 has been PENDING since 06:45 UTC with no completion — roughly 13 hours. That's the job that carries the GIN single-node result, so there is currently no GIN signal on the rebased code. Could someone with MathCI access check whether that build is stuck in queue and re-trigger it?

A new commit is coming shortly for the last open ARGUS thread, which will retrigger all of the above anyway — the coco dispatch is the one that will still need a manual hand.

[dlamd1dai]

@argus re-review

Follow-up on 8eafd3d752 (now f32754f6c1):

Thread	Status
Bind body's comm-wide allgather runs once per logical context	Fixed — the local-base probe moved into ginAnvilRegisterSignalSpan (already per span) and the conn-check moved up to ncclGinAnvilBindResourceWindowSignals, called once after the context loop. The nRanks/lsaSelf warnings moved with them.
Low: dead spanRefs fallback	Fixed — counter and ncclGinAnvilIpcTableUnregister fallback removed
Low: dead gpuCtxHost stores in ginAnvilFreeGinCtx	Fixed — removed

Two regression tests pin the frequency, both verified to fail on 8eafd3d752 and pass on f32754f6c1:

Test	8eafd3d752	f32754f6c1
BindSignals_CommWideCollectivesRunOncePerSpan (nContexts=3, healthy gate)	3 bootstrapAllGather	1
BindSignals_SkippedConnCheckStillRunsOncePerSpan (nContexts=3, gate skips on lsaSize != nRanks)	3 intra-node allgathers	1

Single-node Anvil retest on MI355X (smci355-ccs-aus-m03-17), 1 node / 2 ranks, NCCL_GIN_TYPE=7:

• rccl-UnitTestsGinAnvilPlugin 37/37
• Barrier_AllContextsConsecutiveSignalsDoNotAlias_SingleNode PASSED (668 ms)
• Full GinMPIDeviceTests.* on Anvil: 39 passed, 23 skipped, 0 failed — skips are the 4-rank, cross-node, RAIL, and VA-signal cases

The remaining Low (g_pendingByComm without pluginMutex) is still #12451 on develop, as is the world-size allgather sizing in #12450. Both will need a rebase onto this branch if it lands first.

[amd-argus]

ARGUS

Bottom line: Nothing new worth raising; still open from earlier: gin_plugin_anvil_sdma.cc:768 and 1 more. Worth clearing before approval.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

Rebased onto develop at 482e541856 and force-pushed as cf8cf9720f. The PR is MERGEABLE again.

What changed since the last review pass

• Rebase picked up the ginAnvilCheckSignalConnectivity rework from fix(rccl/gin): conn-check production nits (AICOMRCCL-2218) #11822 (the warnStep lambda, kMaxConnCheckHostRanks, the lastLocalMissing/lastGlobalMissing tracking) and the stub migration from SetConnCheckVerifyMissing to SetConnCheckMissingCalls(-1). Those are kept as develop has them; only the added signalRemoteAddrsDev parameter is mine.
• Folded in the two fixes I had split out as fix(gin): size Anvil signal allgather to the world communicator #12450 and fix(gin): snapshot Anvil pending contexts under pluginMutex #12451, both now closed as superseded. Splitting them stopped making sense after the rebase: this PR deletes ginAnvilRegisterLsaSignals outright, and the pending-list walk it restructures has three unlocked reads rather than the one those patches touched. Details are in the Technical Details section.

Retest (clean worktree at cf8cf9720f, MI355X smci355-ccs-aus-m03-17, 1 node / 2 ranks, NCCL_GIN_TYPE=7)

• rccl-UnitTestsGinAnvilPlugin: 37/37 passed.
• GinMPIDeviceTests.* on the Anvil backend: 44 passed, 20 skipped, 0 failed. The skips are topology or opt-in gated (cross-node, 4-rank, RailConnection_Create, the VASignal_* cases that SDMA does not implement, and the large signal-pool cases). Full breakdown in the Test Result section.

Asks

1. RCCL PR gate is failing only because a fork PR cannot auto-dispatch the coco smoke run (IS_FORK=true, GATE_RESULT=skipped). Could a maintainer dispatch the "RCCL coco PR" workflow with pr_number=11569 and pr_sha=cf8cf9720f16391c76a19534b2af34eaea0c67cd and then override the gate? I cannot trigger it from the fork. MathCI was green on the previous head (precheckin(rccl), extra(rccl), static-analysis(rccl)) and is re-running on this one.
2. This still has no human review. Would appreciate a look from @ROCm/RCCL-Reviewers.

@argus re-review

[amd-argus]

ARGUS

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

Superseded by #12751 and closing in favour of it. Same commits, same content; identical head SHA.

Why. This PR was opened from a fork, and the required RCCL PR gate check can never pass that way. Fork pull requests get no secrets, so the RCCL coco PR workflow skips its Setup job, the coco smoke never runs, and the gate fails closed within seconds. Clearing it here would need a maintainer to dispatch the workflow by hand and then override the check, on every push. #12751 is branched in ROCm/rocm-systems, where the gate dispatches coco to both the Ruby and OCI clusters on its own.

Review history stays here: 27 threads, all resolved. No approval was lost -- this PR had no approval or changes-requested decision -- so the conclusions still apply to #12751.

Closing so this does not keep consuming CI or show up as a duplicate for reviewers; it stays readable and can be reopened.