fix(rccl): forward inherited device options to the dispatcher compile by lfmeadow · Pull Request #12318 · ROCm/rocm-systems

lfmeadow · 2026-09-26T01:03:27Z

JIRA ID : LCOMPILER-2794

The --link step compiles the dispatcher (common.cu.cpp) on its way to
device.elf, so it is a device codegen step like the per-kernel objects it is
linked against, but it was not given ${DL_INHERITED_FLAGS}. Under ASAN that
leaves the dispatcher as the one uninstrumented kernel in an otherwise
instrumented image, since -fsanitize=address reaches device code only through
that variable. The dispatcher also carries the kernel entry, so losing kernarg
preloading there costs a memory round trip at every launch.

Why the forward needs a second list

Forwarding ${DL_INHERITED_FLAGS} as-is fails the device link with 20
relocations of the form R_AMDGPU_REL32_LO cannot be used against symbol ...; recompile with -fPIC. The cause is -fvisibility. The HIP driver adds
-fapply-global-visibility-to-externs to a device compile only when the user
has not passed -fvisibility= itself, and that is what stamps hidden onto the
~3000 ncclDevFunc_* declarations the dispatcher emits. Those declarations are
what keep the symbols non-preemptible, because --compile's extracted assembly
re-exports each definition with a bare .globl.

So the --link step gets its own DL_LINK_INHERITED_FLAGS, which is
DL_INHERITED_FLAGS minus any -fvisibility= option. Two details matter:

The drop is scoped to that one consumer. DL_INHERITED_FLAGS feeds 30
commands in this file, most of them the host fat objects — common,
onerank, collectives, the dda_*, gin_* and sym_* families — that
link into librccl.so. The CMake build has no version script, so taking
-fvisibility=hidden out of the shared list would export rccl internals.
The filter matches any -fvisibility= value, not just =hidden. Clang
suppresses -fapply-global-visibility-to-externs for all of them, and
src/CMakeLists.txt sets =default under ENABLE_CODE_COVERAGE, and under
BUILD_TESTS with either ENABLE_MPI_TESTS or a Debug build on
ROCm >= 6.4, all of which reach here.

The result is purely additive against develop: DL_INHERITED_FLAGS and the
comment block above it are unchanged, and the diff is the new loop plus the
forward.

Tests

Two clean gfx950 builds.

Release: zero relocation errors; ncclDevFunc_* in device.elf is 3064
LOCAL HIDDEN, unchanged from develop; the --link rule in build.ninja
carries -mllvm --amdgpu-kernarg-preload-count=16 and no -fvisibility, while
3382 other commands still carry -fvisibility=hidden; librccl.so has 1212
dynamic symbols and exports none of ncclAllReduce_impl, ncclTopoGetSystem
or bootstrapNetInit.

ASAN (./install.sh --ninja --address-sanitizer --amdgpu_targets gfx950):
-fsanitize=address reaches the --link step, the build completes 3996/3996
with zero relocation errors, and device.elf links.

The ASAN device image is still uninstrumented — flags 0x54f (xnack = ANY),
zero asan symbols — because DeviceLinker.cmake reduces the target to bare
gfx950 before codegen. That is #12394's job, and this PR neither causes nor
fixes it.

Notes for review

One of the PRs replacing the closed #9763 and #12221. No ordering requirement
against the others, though #12394 touches the same file and whichever lands
second will need a rebase.

Update after review

The first version filtered -fvisibility=hidden out of DL_INHERITED_FLAGS
itself. Review caught that this was wrong in both directions: too broad,
because the shared list feeds the host objects described above, and too narrow,
because the alternation matched only the literal =hidden while =default is
live in two configs. Both are fixed by the scoped, value-agnostic list. The
claim that --link was the only codegen step without the flags was also
false — device_ir and the rocSHMEM constmem bitcode compile still lack them —
and has been dropped.

therock-pr-bot · 2026-09-26T01:03:58Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	PR does not contain code files — Unit Test auto-passed
🚫 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?

lfmeadow · 2026-09-26T20:11:56Z

@argus re-review

lfmeadow · 2026-09-26T20:37:45Z

@argus re-review

The --link step compiles the dispatcher (common.cu.cpp) on its way to device.elf, so it is a device codegen step like the per-kernel objects it is linked against, but it was not given DL_INHERITED_FLAGS. Under ASAN that makes the dispatcher the single uninstrumented kernel in an otherwise instrumented image, since -fsanitize=address reaches device code only through that variable. Losing kernarg preloading there costs a memory round trip at the kernel entry the dispatcher carries. Forwarding the list as-is fails the device link with 20 relocations of the form "R_AMDGPU_REL32_LO cannot be used against symbol ...; recompile with -fPIC". The cause is -fvisibility. The HIP driver adds -fapply-global-visibility-to-externs to a device compile only when the user has not passed -fvisibility= itself, and that is what stamps hidden onto the ~3000 ncclDevFunc_* declarations the dispatcher emits. Those declarations are what keep the symbols non-preemptible, because --compile's extracted assembly re-exports each definition with a bare .globl. The drop is therefore scoped to this one consumer, as DL_LINK_INHERITED_FLAGS, rather than taken out of DL_INHERITED_FLAGS itself. That variable feeds 30 commands in this file, most of them the host fat objects (common, onerank, collectives, the dda_*, gin_* and sym_* families) that link into librccl.so; the CMake build has no version script, so dropping -fvisibility=hidden from those would export rccl internals. The filter also matches any -fvisibility= value, not just =hidden: clang suppresses -fapply-global-visibility-to-externs for all of them, and src/CMakeLists.txt sets =default under ENABLE_CODE_COVERAGE and under BUILD_TESTS with ENABLE_MPI_TESTS. Verified by two clean gfx950 builds. Release: zero relocation errors, ncclDevFunc_* in device.elf is 3064 LOCAL HIDDEN, the --link rule carries -mllvm --amdgpu-kernarg-preload-count=16 and no -fvisibility while 3382 other commands keep -fvisibility=hidden, and librccl.so exports none of the internals. ASAN: -fsanitize=address reaches the --link step, zero relocation errors, device.elf links.

amd-argus

ARGUS

Bottom line: LGTM.

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

alex-breslow-amd

LGTM

systems-assistant · 2026-10-05T04:21:08Z

TheRock Submodule Bump Activity

Newest first

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

## Motivation `--enable-full-coverage` / `ENABLE_FULL_COVERAGE` (#10650) no longer instruments any device code on `develop`. The NCCL v2.31.2-1 sync merge (`d0f7d1966a5`, #10785) dropped the device-coverage parts of #10650 from `src/CMakeLists.txt` and `cmake/DeviceLinker.cmake`. CMake still reports `ENABLE_FULL_COVERAGE=ON`, but: - no `device-<arch>.elf` is produced next to `librccl.so` - `librccl.so` has no `__llvm_profile_hip_collect_device_data` reference - so `llvm-cov` reports 0% on every device kernel The other files #10650 touched (`DeviceCoverage.cmake`, top-level `CMakeLists.txt`, `install.sh`, `rccl-device-compile`, test runner) survived the merge and are unchanged here. ## Technical Details This cherry-picks `0beb60252bd` (#10650) and resolves it against current `develop`: - **Files that survived the merge intact:** kept develop's version (CHANGELOG, `rccl-device-compile`, test runner files). - **`src/CMakeLists.txt`:** the `ENABLE_CODE_COVERAGE` block is restored verbatim from #10650. This brings back the host/device flag split and the force-link of `__llvm_profile_hip_collect_device_data` plus the host ROCm profile runtime. - **`cmake/DeviceLinker.cmake`:** - Restores the coverage flags, device profile runtime, main-module CUID and device-ELF symlink plumbing. - The `--link` step keeps develop's `DL_LINK_INHERITED_FLAGS` (#12318) alongside `DL_COVERAGE_FLAGS` and `DL_MAIN_MODULE_CUID_FLAGS`. - The bundle `DEPENDS` keeps develop's longer object list. - The diagnostics `p2p.cu.cpp` fat object, added by the sync after #10650, now gets `DL_DEVICE_COVERAGE_FLAGS` like its sibling fat objects. Otherwise its device code would be instrumented through the inherited flags without the device profile runtime. **Second commit — host ROCm profile runtime lookup.** With the hooks restored, the instrumented `librccl.so` still failed to load on TheRock 10.1: ``` librccl.so.1: undefined symbol: _ZN14__interception17InterceptFunctionEPKcPmmm ``` - `rccl_find_host_rocm_profile_runtime` only asked the compiler for `libclang_rt.profile_rocm.a`, the per-target runtime layout. - TheRock 10.1 ships the legacy `lib/linux/libclang_rt.profile_rocm-x86_64.a`, so the lookup returned empty. - The forced `__llvm_profile_hip_collect_device_data` reference then resolved from the generic `libclang_rt.profile-x86_64.a`. Its ROCm collector object references `__interception::InterceptFunction` without defining it. The lookup now tries the arch-suffixed name after the per-target one. A new test, `test_host_probe_finds_legacy_arch_suffixed_runtime`, covers this and fails without the fix. No behaviour change without `ENABLE_FULL_COVERAGE`: every restored line is gated on it. ## Issue Tracking JIRA ID: AICOMRCCL-2318 ## Test Plan Ruby (MI350X, gfx950), TheRock ROCm 10.1. Same command on this branch and on `develop`: ```bash ./install.sh --debug -t --enable-full-coverage --ninja --amdgpu_targets=gfx950 ``` Then inspect the build and run `rccl-UnitTests --gtest_filter='AllReduce.*:AllGather.*:Broadcast.*'` with `LLVM_PROFILE_FILE` set, merge, and `llvm-cov report librccl.so -object device-gfx950.elf`. ## Test Result | `--enable-full-coverage`, TheRock 10.1, gfx950 | `develop` | this PR | | --- | --- | --- | | Build | pass | pass | | `ENABLE_FULL_COVERAGE` in CMakeCache | ON | ON | | `device-gfx950.elf` produced | **no** | yes | | device ELF sections | — | `__llvm_prf_cnts __llvm_prf_data __llvm_covmap __llvm_covfun` | | `__llvm_profile_hip_collect_device_data` in `librccl.so` | 0 | 1 | - `tools/scripts/test_runner` unit suite: 112/112 pass (`python3 -m unittest discover -s tests -t .`; needs an exec-capable `TMPDIR`, because the fake-compiler tests fail on a `noexec` `/tmp`). - Runtime device coverage from the gtest subset: results to follow in a comment. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Signed-off-by: Praneeth Vallem <Praneeth.Vallem@amd.com> Co-authored-by: Atul Kulkarni <atul.kulkarni@amd.com>

…eline (#12394) JIRA ID : LCOMPILER-2794 Replaces #12313, which was closed by accident. Same changes, rebased onto current develop. Feature suffixes were being lost in the device linker pipeline. This caused features like `xnack+` to be lost when compiling for ASAN. ## What changed These changes keep both the bare processor name and the feature string. One site outside the device linker pipeline also dropped the feature string: the rocSHMEM build. This is also fixed. ## Tests New cases in the canonical driver suite at `tools/scripts/test_runner/tests/`. ## Notes for review One of the PRs replacing the closed #9763 and #12221. Nothing here depends on the others, though #12318 touches the same file and whichever lands second will need a rebase. ASAN device code needs both this and #12314: the latter decides *which* targets an ASAN build asks for, and this PR makes the request reach codegen. They can land in either order, but kernels are only instrumented once both are in. Verified with clean builds at `gfx950` and `gfx942:xnack+`, plus a 3-arch ASAN build, on ROCm clang 23 and clang 24. After the rebase, re-verified with clean `gfx950` ASAN builds, one with tests and one with `--rocshmem`: no `ignoring '-fsanitize=address'` warnings. --------- Co-authored-by: Cursor <cursoragent@cursor.com>

lfmeadow requested review from a team and a lite review from Copilot September 26, 2026 01:03

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

Copilot started reviewing on behalf of lfmeadow September 26, 2026 01:04

View session

This comment was marked as resolved.

Sign in to view

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

This comment was marked as outdated.

Sign in to view

lfmeadow force-pushed the users/lmeadows/dl-link-inherited-flags branch 2 times, most recently from 93fcf59 to c2b2c5c Compare September 26, 2026 19:21

lfmeadow mentioned this pull request Sep 26, 2026

fix(rccl): carry the full GPU target ID through the device-linker pipeline #12313

Closed

lfmeadow requested a lite review from Copilot September 26, 2026 20:11

Copilot started reviewing on behalf of lfmeadow September 26, 2026 20:12

View session

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

lfmeadow requested a lite review from Copilot September 26, 2026 20:37

Copilot started reviewing on behalf of lfmeadow September 26, 2026 20:38

View session

This comment was marked as resolved.

Sign in to view

lfmeadow force-pushed the users/lmeadows/dl-link-inherited-flags branch from 6d4dcb8 to 4af6314 Compare September 26, 2026 20:44

This comment was marked as outdated.

Sign in to view

lfmeadow force-pushed the users/lmeadows/dl-link-inherited-flags branch from 4af6314 to 8ee6992 Compare September 28, 2026 01:53

lfmeadow force-pushed the users/lmeadows/dl-link-inherited-flags branch from 8ee6992 to a61a4c0 Compare September 28, 2026 03:49

amd-argus Bot reviewed Sep 28, 2026

View reviewed changes

lfmeadow mentioned this pull request Sep 28, 2026

fix(rccl): carry the full GPU target ID through the device-linker pipeline #12394

Merged

alex-breslow-amd approved these changes Sep 29, 2026

View reviewed changes

corey-derochie-amd approved these changes Sep 29, 2026

View reviewed changes

lfmeadow merged commit 2eff2c6 into develop Sep 29, 2026
83 of 88 checks passed

lfmeadow deleted the users/lmeadows/dl-link-inherited-flags branch September 29, 2026 15:58

pvallem mentioned this pull request Oct 2, 2026

fix(rccl): restore device code coverage on develop #12695

Merged

1 task done

Navigation Menu

Repository navigation

fix(rccl): forward inherited device options to the dispatcher compile - #12318

fix(rccl): forward inherited device options to the dispatcher compile#12318
lfmeadow merged 1 commit into
developfrom
users/lmeadows/dl-link-inherited-flags

lfmeadow commented Sep 26, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

lfmeadow commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

lfmeadow commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

amd-argus Bot left a comment

Uh oh!

alex-breslow-amd left a comment

Uh oh!

Uh oh!

systems-assistant Bot commented Oct 5, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

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

Uh oh!

Why the forward needs a second list

Tests

Notes for review

Update after review

Uh oh!

therock-pr-bot Bot commented Sep 26, 2026

✅ All Policy Checks Passed

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

lfmeadow commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

lfmeadow commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

alex-breslow-amd left a comment

Choose a reason for hiding this comment

Uh oh!

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!

4 participants

lfmeadow commented Sep 26, 2026 •

edited

Loading