[aduforest]

JIRA ID: AICOMRCCL-2358

What

Adds host-only microtests for src/ras/ras.cc, covering every executable source line and function without changing production code.

The 71 tests cover:

• message and collective length calculation;
• message allocation, release, and send-queue insertion;
• complete, partial, resumed, closed, and failed send/receive progress;
• poll-slot allocation, reuse, initialization, and growth;
• communicator initialization, registration, removal, and cleanup;
• local add-ranks and termination notifications;
• connection-init and acknowledgement handling, including NACK and peer-update paths;
• dispatch of keepalive, peer-update, collective-request, collective-response, and dead-peer messages;
• RAS event-loop dispatch, timeout handling, interrupted/invalid poll handling, cleanup, and termination.

No production source is changed. Live multi-process/network integration remains the responsibility of RCCL integration tests; these tests execute the RAS logic through host-only seams.

How

ras-test.cc includes the hipified ras/ras.cc directly, following the existing host-microtest model. External network, peer, collective, client, socket-pair, and event-loop operations are TU-local seams because this is their sole test consumer, following the single-consumer rule established by #11647.

Coverage

• Functions: 21/21 (100%)
• Lines: 468/468 (100%)
• Regions: 1090/2117 (51.49%)
• Branches: 503/1061 (47.41%)
• Original focused mutations: 6/6 killed

Test plan

Check	Result
RasMicrotest.* (current develop, local)	71/71 passed
RasMicrotest.* --gtest_shuffle --gtest_repeat=20	71/71 × 20 passed
Full rccl-UnitTestsMicro	pending fresh CI; previous head passed 447/447
Coverage report 468/468 lines and 21/21 functions
git diff --check	passed
Production source changes	none

[amd-argus]

ARGUS

Findings

1 finding(s) that could not be posted as inline comments

• projects/rccl/test/host/CMakeLists.txt:44 (Nit) The link-error guarantee at CMakeLists.txt:43-45 is narrowed by the new --gc-sections The comment at test/host/CMakeLists.txt:43-45 still states without qualification that any un-shimmed HIP call surfaces as a link error, and the -Wl,--gc-sections added at line 105 narrows that to reached paths only, since a symbol referenced solely from a discarded section is never reported. Lines 304-315 of the same file already record this empirically for the Enqueue target. The practical effect is small, because a call from live code still errors and the same trade-off is already accepted on rccl-UnitTestsMicroInit and rccl-UnitTestsMicroEnqueue, so narrowing 43-45 to say on a reached path would be enough to keep it honest for the five pre-existing TUs on this target.

This PR changes 1,309 lines across 4 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: CommInitReusesVacantCommSlot and CommFiniRemovesMatchingCommAndDropsReference each hold two ~3.8 MB ncclComm stack locals, about 7.6 MB of an 8 MB stack.

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

[amd-argus]

ARGUS

This PR changes 1,378 lines across 5 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: the dead-peer NACK at ras.cc:429 and both listeningAddr memcpys can each be deleted with all 58 tests still green.

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

[amd-argus]

ARGUS

This PR changes 1,474 lines across 5 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: ras.cc:209 has no reachable test, so the 439/439 line figure does not hold, and nine fakes discard the arguments production chose.

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

[amd-argus]

ARGUS

Findings

2 finding(s) that could not be posted as inline comments

• projects/rccl/test/host/ras-test.cc:829 (Medium) ConnInitCreatesReadyConnectionAndQueuesAck takes the peerIdx == -1 arm without observing it ConnInitCreatesReadyConnectionAndQueuesAck runs with g_peerFindResult at its default of -1, so it takes the peerIdx == -1 arm at projects/rccl/src/ras/ras.cc:521 without observing it: deleting that guard entirely still passes, because the only g_linkUpdateCalls assertion is the EXPECT_EQ(2, ...) at line 848, in the one test that sets g_peerFindResult to 3. Three zero-assertions here pin that arm and the two other untaken paths alongside it.
• projects/rccl/test/host/ras-test.cc:997 (Medium) rasSocketsHead is a single head-matching socket, so the rasSockets search loop never iterates rasSocketsHead is a single head-matching socket, so the search loop at projects/rccl/src/ras/ras.cc:687-694 never iterates: deleting the break at ras.cc:691, replacing if (sock == nullptr) at ras.cc:696 with if (true), and truncating the loop to the head alone all leave the assertions at lines 1032 to 1037 passing. The client list right below already does this properly, with a deliberate non-matching head at line 998.

This PR changes 1,474 lines across 5 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: ncclRasCommInit is never called with myRank == nullptr, the shape bootstrap.cc:1401 uses, so the guard at ras.cc:143 can be deleted with all 62 tests green.

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

[aduforest]

@argus re-review

[amd-argus]

ARGUS

This PR changes 1,722 lines across 5 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

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

[amd-argus]

ARGUS

This PR changes 1,722 lines across 5 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Nothing new worth raising; still open from earlier: ras-test.cc:1414. Worth clearing before approval.

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

[systems-assistant]

TheRock Submodule Bump Activity

Newest first

• 2026-09-19 — Included in TheRock via ROCm/TheRock#8334.