test(rccl): add host microtests for nccl_device/utility.h math helpers by aduforest · Pull Request #13315 · ROCm/rocm-systems

aduforest · 2026-10-12T21:54:53Z

JIRA ID: AICOMRCCL-2825

What

Host-only microtests for the host/device math helpers in src/include/nccl_device/utility.h: rounding/alignment, rolling counters, the fast-division reciprocals, memory-order conversion, add4G and Optional/Present. Second PR in the AICOMRCCL-2825 stack.

How

Header-only, so the tests go in rccl-UnitTestsMicro (nccl-utility-test.cc, suite NcclUtilityMicrotest.*). No fakes.

Coverage

40/40 functions, 186/186 lines, 113/113 regions, 62/62 branches.

Mutation Testing

15 mutations on the hipified header, all killed.

Test Plan

NcclUtilityMicrotest.*: 27/27, and 27/27 × 20 shuffled.
Full rccl-UnitTestsMicro on develop: 1930/1930.
Test-only change; no production source touched.

🤖 Generated with Claude Code

therock-pr-bot · 2026-10-12T21:55:41Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like `test_<name>.py` / `test_<name>.cpp` (or `<name>_test.`). Current:* code file(s) changed: `projects/rccl/test/host/nccl-utility-test.cc`; no test file found
🚫 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?

aduforest · 2026-10-12T23:10:02Z

@argus review

amd-argus

ARGUS

Bottom line: Worth settling before approval: rollingLessEq's nBits mask is never exercised though production passes 32, and the idivFast64 truncation the case name promises is never reached.

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

aduforest · 2026-10-13T05:26:41Z

@argus review

aduforest · 2026-10-13T07:28:54Z

@argus re-review

amd-argus

ARGUS

Bottom line: Worth settling before approval: the alignUp pointer test still passes under the (addr + a - 1) off-by-one it was revised to catch, because buf + 3 rounds to buf + 16 either way.

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

aduforest · 2026-10-13T19:11:38Z

@argus review

amd-argus

ARGUS

Bottom line: Looks approval ready: everything still open is comment accuracy or a latent include hazard, none of it behavioural.

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

aduforest · 2026-10-13T21:35:30Z

@argus review

amd-argus

ARGUS

Bottom line: Worth settling before approval: three assertions pass under a one-character mutation of the helper they cover, in imulRcp32's rem negation, rollingLessEq's half-range constant, and alignDown's mask.

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

- rollingLessEq: add a partial-width (nBits=32 on a uint64_t) pair; every existing call passed nBits == full width, leaving the mask shift dead. - idivFast64: add a quotient above 2^32 (1<<40 / 1); the existing case's quotient fit in 32 bits, so the truncation it claimed to test was inert. - AlignUp/AlignDown pointer overload: pin exact addresses instead of mod/bound checks that an off-by-one in the production formula also passes. - roundUp/roundDown: add a non-power-of-two modulus pair (3), since modulus-8-only can't tell % from a power-of-two mask. - idivRcp32/64 sweeps: add 0xFFFFFFFFu / ~0ull to catch an over-estimating reciprocal, which the existing +1 under-estimate correction can't. - Remove a tautological loop in the rolling-comparator test: it reduced to the same call on both sides regardless of a swap bug, so it could not fail. - Fix two comments: the IdivRcp32 pow2-correction is a branch-avoidance optimisation, not required for round-trip correctness; and the 'no HIP/device-only dependency' header/README claim doesn't hold for the four memory_order helpers (true only because this build's --offload-host-only path still invokes hipcc). - Two self-referential assertions (mul32hi/mul64hi) replaced with literal expected values. - present_opt -> presentOpt naming nit.

…wrapper sweeps Co-Authored-By: Claude <noreply@anthropic.com>

…tops.h using-declaration trap Co-Authored-By: Claude <noreply@anthropic.com>

…dary, and imulRcp's rem negation Co-Authored-By: Claude <noreply@anthropic.com>

aduforest · 2026-10-14T03:01:03Z

@argus review

aduforest · 2026-10-14T22:11:49Z

@argus review

amd-argus

ARGUS

Bottom line: LGTM.

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

…tests Shorten the MICROTEST_README entry, CMake comments and test-file comments to the length the other host microtests use. Comment and doc changes only. Co-Authored-By: Claude <noreply@anthropic.com>

aduforest · 2026-10-14T23:43:15Z

@argus review

amd-argus

ARGUS

Bottom line: LGTM.

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

aduforest · 2026-10-15T19:27:25Z

Folded into #13312 (one PR per Jira ticket, so AICOMRCCL-2825 runs CI once). Same commits, unchanged.

github-actions Bot added the project: rccl label Oct 12, 2026

aduforest mentioned this pull request Oct 12, 2026

test(rccl): add host microtests for nccl_device/core.cc team helpers #13316

Closed

systems-assistant Bot added the organization: ROCm label Oct 12, 2026

aduforest mentioned this pull request Oct 12, 2026

test(rccl): add host microtests for src/cft_dev_runtime.cc #13320

Closed

amd-argus Bot reviewed Oct 13, 2026

View reviewed changes

aduforest mentioned this pull request Oct 13, 2026

test(rccl): add host microtests for the ROCm runtime and SMI wrappers #13335

Merged

amd-argus Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

aduforest force-pushed the users/aduforest/aicomrccl-2825-utility-host-microtests branch from ad192c7 to 2ddae5d Compare October 13, 2026 19:11

amd-argus Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

amd-argus Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

Comment thread projects/rccl/test/host/nccl-utility-test.cc Resolved

aduforest and others added 5 commits October 13, 2026 20:00

test(rccl): add host microtests for nccl_device/utility.h math helpers

f81f407

test(rccl): pin alignUp's - 1, rollingLessThan's nBits forward, trim …

47a1255

…wrapper sweeps Co-Authored-By: Claude <noreply@anthropic.com>

test(rccl): correct the imulRcp32 counterexample comment, note the bi…

d1eb14c

…tops.h using-declaration trap Co-Authored-By: Claude <noreply@anthropic.com>

test(rccl): pin alignDown's negation, rollingLessEq's half-range boun…

87d7310

…dary, and imulRcp's rem negation Co-Authored-By: Claude <noreply@anthropic.com>

aduforest force-pushed the users/aduforest/aicomrccl-2825-utility-host-microtests branch from 8bc102a to 87d7310 Compare October 14, 2026 03:00

amd-argus Bot reviewed Oct 14, 2026

View reviewed changes

test(rccl): trim microtest docs and comments to match the other host …

f41e1d5

…tests Shorten the MICROTEST_README entry, CMake comments and test-file comments to the length the other host microtests use. Comment and doc changes only. Co-Authored-By: Claude <noreply@anthropic.com>

aduforest marked this pull request as ready for review October 14, 2026 23:45

aduforest requested review from a team October 14, 2026 23:45

amd-argus Bot reviewed Oct 15, 2026

View reviewed changes

aduforest mentioned this pull request Oct 15, 2026

test(rccl): add host microtests for the device API host side #13312

Merged

aduforest closed this Oct 15, 2026

Conversation

aduforest commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Coverage

Mutation Testing

Test Plan

Uh oh!

therock-pr-bot Bot commented Oct 12, 2026

✅ All Policy Checks Passed

Uh oh!

aduforest commented Oct 12, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduforest commented Oct 13, 2026

Uh oh!

aduforest commented Oct 13, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduforest commented Oct 13, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aduforest commented Oct 13, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduforest commented Oct 14, 2026

Uh oh!

aduforest commented Oct 14, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aduforest commented Oct 14, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aduforest commented Oct 15, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aduforest commented Oct 12, 2026 •

edited

Loading