fix(rccl/gin): conn-check production nits (AICOMRCCL-2218) by dlamd1dai · Pull Request #11822 · ROCm/rocm-systems

dlamd1dai · 2026-09-18T01:32:27Z

Summary

Follow-up to ROCm/rocm-systems#11092 (AICOMRCCL-2217), now rebased onto develop after that PR merged. Production-only conn-check behaviour hardening:

Split kMaxConnCheckKernelRanks (1024) from kMaxConnCheckHostRanks (IPC 16) in gin_anvil_conn_check.h with static_assert
Null-check conn-check launcher inputs before HIP launch
Collapse duplicated step-failure WARNs behind a warnStep lambda
Distinguish local vs peer-reported connectivity failures in retry/final diagnostics (global missing is summed over the LSA team)
Fold SetConnCheckVerifyMissing into SetConnCheckMissingCalls (negative = always missing)
Argus follow-ups: assert pre-launch rejects leave hipSuccess; collapse peer/global retry WARN; clarify stub comment for zero
Step-failure WARN names the failed step and does not print the allgather abort sentinel as a missing count
Reject an out-of-range selfRank before ginAnvilConnWrite launches

Comm-reuse erase is not here: ROCm/rocm-systems#12218 (AICOMRCCL-2455).

Test plan

rccl-UnitTestsGinAnvilPlugin --gtest_filter='*ConnCheck*' — passed 15/15 on smci350-rck-g03-d07-21 (MI350X, gfx950)
rccl-UnitTestsGinAnvilConnCheckDevice (null-pointer + rank-cap accept/reject + selfRank reject + no HIP error on reject) — passed 6/6 on smci350-rck-g03-d07-21 (MI350X, gfx950)

JIRA ID: AICOMRCCL-2218

amd-argus

ARGUS

Bottom line: LGTM once RejectsTooManyRanksBeforeLaunch also asserts no launch happened. That one is mechanical, so it does not need another pass from me.

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

Address Argus nits on #11822 and move the conn-checked set erase from unreachable plugin finalize into closeColl so a recycled ncclComm* cannot skip the gate after HostFinalize. Co-authored-by: Cursor <cursoragent@cursor.com>

dlamd1dai · 2026-09-22T23:12:19Z

Pushed acd858655f5:

Argus fix(rccl/gin): conn-check production nits (AICOMRCCL-2218) #11822 nits: pre-launch rank/null rejects clear/peek HIP error; peer vs global retry WARN collapse; stub comment documents zero = never.
Dedup-set erase moved from ginAnvilFinalize → ginAnvilCloseColl, with ConnCheck_CloseCollUnmarksCommForReuse (closeColl without finalize, second bind must re-run the gate). Addresses the recycled-ncclComm* skip that Argus flagged on fix(rccl/gin-sdma): restore LSA conn-check with device-linked TU (AICOMRCCL-2217) #11092; keeping that fix off fix(rccl/gin-sdma): restore LSA conn-check with device-linked TU (AICOMRCCL-2217) #11092 per stack split.

Still blocked on #11092. Will rebase onto develop after that lands.

amd-argus

ARGUS

Bottom line: LGTM once the step-failure case stops printing the lsaTeamSize sentinel as local missing at gin_plugin_anvil_sdma.cc:699. That one is mechanical, so it does not need another pass from me.

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

dlamd1dai · 2026-09-24T18:00:58Z

Comm-reuse closeColl erase + reuse gtest moved to #12218 (AICOMRCCL-2455) so this PR stays 2218 production nits only.

Address Argus nits on #11822 and move the conn-checked set erase from unreachable plugin finalize into closeColl so a recycled ncclComm* cannot skip the gate after HostFinalize. Co-authored-by: Cursor <cursoragent@cursor.com>

amd-argus

ARGUS

Bottom line: Worth settling before approval: the gate-failure WARN at gin_plugin_anvil_sdma.cc:694 still prints the lsaTeamSize sentinel as a measured local missing count.

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

Split device vs host rank caps in gin_anvil_conn_check.h, null-check conn-check launch inputs, fold verify-missing into the missing-call counter, and tighten step/peer failure diagnostics. Stacked on users/dondai/aicomrccl-2217-conn-check-restore (#11092). Co-authored-by: Cursor <cursoragent@cursor.com>

Address Argus nits on #11822 and move the conn-checked set erase from unreachable plugin finalize into closeColl so a recycled ncclComm* cannot skip the gate after HostFinalize. Co-authored-by: Cursor <cursoragent@cursor.com>

Comm-reuse un-mark lives on stacked PR for AICOMRCCL-2455 so this PR stays production nits only.

…inel A local step failure sets localMissing to the team size only so the allgather aborts every rank. Logging that value as a measured missing count hides the failed step. Co-authored-by: Cursor <cursoragent@cursor.com>

amd-argus

ARGUS

Bottom line: LGTM.

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

abouteiller

LGTM

systems-assistant · 2026-10-05T04:20:14Z

TheRock Submodule Bump Activity

Newest first

2026-10-05 — Included in TheRock via ROCm/TheRock#8732.

dlamd1dai requested review from a team as code owners September 18, 2026 01:32

This was referenced Sep 18, 2026

fix(rccl/gin): conn-check production nits (AICOMRCCL-2218) dlamd1dai/rocm-systems#13

Closed

fix(rccl/gin-sdma): restore LSA conn-check with device-linked TU (AICOMRCCL-2217) #11092

Merged

github-actions Bot added the project: rccl label Sep 18, 2026

systems-assistant Bot added the organization: ROCm label Sep 18, 2026

amd-argus Bot reviewed Sep 18, 2026

View reviewed changes

Comment thread projects/rccl/test/gin/gin_anvil_conn_check_device_test.cpp Resolved

Comment thread projects/rccl/src/gin/gin_plugin_anvil_sdma.cc Resolved

Comment thread projects/rccl/test/gin/gin_anvil_plugin_test_stubs.h Resolved

dlamd1dai self-assigned this Sep 18, 2026

amd-argus Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread projects/rccl/src/gin/gin_plugin_anvil_sdma.cc Resolved

dlamd1dai mentioned this pull request Sep 24, 2026

fix(rccl/gin): erase conn-check dedup on closeColl (AICOMRCCL-2455) #12218

Merged

1 task done

dlamd1dai force-pushed the users/dondai/aicomrccl-2217-conn-check-restore branch from c21fcd0 to 4392d1c Compare September 24, 2026 18:24

dlamd1dai force-pushed the users/dondai/aicomrccl-2218-conn-check-nits branch from 137181b to bd12427 Compare September 24, 2026 19:29

amd-argus Bot reviewed Sep 24, 2026

View reviewed changes

Comment thread projects/rccl/src/gin/gin_anvil_conn_check_device.cc Resolved

Comment thread projects/rccl/test/gin/gin_anvil_conn_check_device_test.cpp Resolved

dlamd1dai and others added 3 commits September 28, 2026 15:15

fix(rccl/gin): move closeColl erase off 2218 nits (AICOMRCCL-2455)

adc0aa1

Comm-reuse un-mark lives on stacked PR for AICOMRCCL-2455 so this PR stays production nits only.

dlamd1dai force-pushed the users/dondai/aicomrccl-2218-conn-check-nits branch from bd12427 to adc0aa1 Compare September 28, 2026 19:16

dlamd1dai requested review from a team, atgutier, dayatsin-amd, kentrussell, prbasyal-amd and saleelk as code owners September 28, 2026 19:16

dlamd1dai removed request for a team, AryanSalmanpour, atgutier, dayatsin-amd, jrmadsen, kentrussell, kiritigowda, rrawther and saleelk September 28, 2026 19:22

amd-argus Bot reviewed Sep 28, 2026

View reviewed changes

abouteiller approved these changes Sep 29, 2026

View reviewed changes

Kapil-Shyam-Pawar approved these changes Sep 30, 2026

View reviewed changes

dlamd1dai mentioned this pull request Oct 1, 2026

fix(gin): de-alias Anvil signals across logical contexts #11569

Closed

1 task done

Navigation Menu

Repository navigation

fix(rccl/gin): conn-check production nits (AICOMRCCL-2218) - #11822

fix(rccl/gin): conn-check production nits (AICOMRCCL-2218)#11822
dlamd1dai merged 4 commits into
developfrom
users/dondai/aicomrccl-2218-conn-check-nits

dlamd1dai commented Sep 18, 2026 •

edited

Loading

Uh oh!

amd-argus Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlamd1dai commented Sep 22, 2026

Uh oh!

amd-argus Bot left a comment

Uh oh!

Uh oh!

dlamd1dai commented Sep 24, 2026

Uh oh!

amd-argus Bot left a comment

Uh oh!

Uh oh!

Uh oh!

amd-argus Bot left a comment

Uh oh!

abouteiller left a comment

Uh oh!

systems-assistant Bot commented Oct 5, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dlamd1dai commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlamd1dai commented Sep 22, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dlamd1dai commented Sep 24, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

abouteiller left a comment

Choose a reason for hiding this comment

Uh oh!

systems-assistant Bot commented Oct 5, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dlamd1dai commented Sep 18, 2026 •

edited

Loading