[alex-breslow-amd]

Summary

• Retarget of the scoped-atomics work onto rccl-develop (see also fix(rccl): replace deprecated __hip_atomic builtins with __scoped_atomic #11316 against develop).
• Device atomics now use Clang __scoped_atomic_* / __atomic_* instead of __hip_atomic_*, with explicit memory scopes.
• Adds a compile-time poison (RCCL_POISON_HIP_ATOMICS, default ON) and a host-test CI probe that __hip_atomic_load and the rest of the __hip_atomic_* builtins fail to compile in RCCL sources.

src/algorithms/gin/sdma/all_reduce_gin_sdma.h is gone on rccl-develop, so those five call-site edits from the develop PR are omitted here.

Remaining __hip_atomic_* coverage

There are no remaining __hip_atomic_* call sites in RCCL sources after this PR. The only remaining hits are the poison header, the CI probe, and comments.

HIP wrapper APIs that still go through __hip_atomic_* inside HIP headers are still present. Those compile with the poison because HIP parses them before the pragma, so they are the remaining way __hip_atomic_* can still land in librccl.so:

API	Where
atomicAdd	prims_ll.h, prims_simple.h, gin_anvil_sdma.h, gin_rocshmem_gda.h, llx_latency_bench.c
atomicMax	prims_simple.h
atomicCAS	DOCA verbs headers under src/transport/net_ib/gdaki/

Closing that gap means porting those wrappers to __scoped_atomic_* with explicit scopes. I will do that as a separate PR.

Test plan

• ./install.sh -l still builds with poison enabled
• python3 projects/rccl/src/include/test_poison_hip_atomics.py (or test/host/run_host_tests.sh guards)
• Confirm a deliberate __hip_atomic_load in a device TU fails with attempt to use a poisoned identifier

[mch]

Code wise this looks good. I wonder if there is any potential performance risk here though, or risk of older compilers / versions of ROCm that don't support __scoped_atomic_* that RCCL is supposed to support.

[alex-breslow-amd]

Code wise this looks good. I wonder if there is any potential performance risk here though, or risk of older compilers / versions of ROCm that don't support __scoped_atomic_* that RCCL is supposed to support.

I'll check with the compilers folks. This is a clang builtin. It's been there at least as far back as October of 2025. I saw @lfmeadow use it back then. I'll check with him.

It looks like they were added in clang15. We're on clang20. Let me check which version of ROCm used clang14. Update: It looks like we moved to clang 15 for ROCm 5.4. I think this is fine.

[amd-argus]

ARGUS

Bottom line: Worth settling before approval: the poison probe exits 0 when no HIP compiler is found (test_poison_hip_atomics.py:117), so the guard can pass without ever probing. The 37 call-site conversions themselves check out, with no scope or memory-order drift.

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

[alex-breslow-amd]

@amd-argus thanks — the bottom line was right, and worse than the comment says. Pushed f3e6526 addressing all 12 threads; replies are inline on each.

The guard could pass without probing

Confirmed: _find_cxx() returns None, the blanket @unittest.skipUnless skips all six tests, and unittest.main() exits 0. To see how far it goes I gutted poison_hip_atomics.h down to a bare include guard and hid the compiler — the guard still printed OK (skipped=6) and exited 0. A fully disarmed poison reported success.

The fix has two parts. A missing HIP compiler is now a failure rather than a skip (ToolchainTest), with RCCL_POISON_TEST_ALLOW_SKIP=1 to opt out when running locally without ROCm. And the checks that need no compiler — header present, pragma contents — moved into their own PoisonHeaderTest, so they still gate on a toolchain-less box instead of being swept up by the skip.

After the change, the same no-compiler run fails:

Ran 9 tests in 0.000s
FAILED (failures=1, skipped=6)     exit 1

and the gutted-header case now trips two tests, because the pragma check no longer depends on a compiler.

One finding extended

The set -e problem in do_guards was also present one level up in do_run: do_host_tests "$@" followed by do_guards, so a gtest failure dropped both guards entirely. Both now collect with || rc=1 and return rc.

Two notes back

.pre-commit-config.yaml is not the safety net it looks like for the formatting thread. It pins mirrors-clang-format v18.1.4, but projects/rccl/.clang-format uses three clang-format 21-only keys (AlignPPAndNotPP, OneLineFormatOffRegex, BreakBeforeCloseBracketBracedList), so both 18 and ROCm's own 20 refuse to parse the config outright. Pre-existing and out of scope here, but worth knowing.

On the compare_exchange thread I added the probe you asked for, but flagged in its docstring that it pins the builtin rather than hip_compat.h — it would not catch a weak/memory-order swap, since both convert to int. That gap needs a runtime test of atomic_ref, which has no host harness today.

Test evidence

• Poison guard: 9 tests pass. Six negative scenarios each fail as intended — no compiler; gutted header; name dropped from POISONED; name dropped from the pragma; broken _call_expr arity; and RCCL_POISON_TEST_ALLOW_SKIP=1 still exits 0.
• run_host_tests.sh guards end to end: 22 kernel-count + 9 poison tests, exit 0. Stubbed-failure runs confirm both guards report rather than short-circuit.
• CMake configures with the poison on; --include=...poison_hip_atomics.h lands in the rccl target's CXX_FLAGS.
• Compiled real device TUs with the poison active — common.cu.cpp, ce_reduce.cc, and common.h + all_gather.h / reduce_scatter.h (which pull in primitives.h → prims_ll.h and op128.h). A deliberate __hip_atomic_load injected into a device TU fails with attempt to use a poisoned identifier, which is the PR's own test-plan item.
• All eight reformatted headers verified token-identical to the previous revision by whitespace-normalized comparison.
• black --check and clang-format both clean on the changed files.

[alex-breslow-amd]

@argus re-review