Skip to content

fix(rccl): compile rccl_arch_thresholds.cc into topo_expl once - #13706

Merged
alex-breslow-amd merged 2 commits into
developfrom
users/dondai/rccl-topo-expl-dup-arch-thresholds
Oct 20, 2026
Merged

alex-breslow-amd merged 2 commits into
developfrom
users/dondai/rccl-topo-expl-dup-arch-thresholds

Conversation

@dlamd1dai

Copy link
Copy Markdown
Contributor

Motivation

mci/rocm-libraries/extra(rccl) has failed on develop since build #592 (09d8fb17ac), and on every PR that runs it. The topo_expl link fails with:

ld.lld: error: duplicate symbol: rcclGetArchThresholds(char const*, rcclTuningPartition)
>>> defined at .../tools/topo_expl/../../src/rccl_arch_thresholds.cc:540
>>> defined at .../tools/topo_expl/hipify_rccl/rccl_arch_thresholds.cc:540
make: *** [Makefile:146: topo_expl] Error 1

Technical Details

Two PRs that merged on Oct 16 fixed the same missing rcclGetArchThresholds() link in topo_expl:

With both in place, the file is compiled twice. This PR keeps the hipified copy, which matches how rccl_wrap.cc and the other staged sources are handled, and removes the direct entry. The comment from #12336 that explains why the file is needed still applies.

Issue Tracking

N/A (CI break on develop).

Test Plan

Ran Jenkins' build step (serial make in tools/topo_expl) on an MI300X node with ROCm 7.2, once on unmodified develop (b8d1009e44) and once with this change. Then ran the built binary the way the CI test step does (./topo_expl with no arguments), plus two topology models.

Test Result

  • Unmodified develop: make fails with the duplicate rcclGetArchThresholds symbol shown above (rc 2), the same error as math-ci.
  • This PR: topo_expl links. ./topo_expl, ./topo_expl -m 0 and ./topo_expl -m 54 (4x gfx942) all exit 0 and finish generating the topology.

Submission Checklist

#13466 and #12336 each fixed the missing rcclGetArchThresholds() link in
topo_expl: the first through a hipified copy (hipify_rccl/), the second
by compiling ../../src/rccl_arch_thresholds.cc directly. With both merged
the file is compiled twice and the link fails with a duplicate symbol,
which has kept math-ci extra(rccl) red on develop since 09d8fb1.

Keep the hipified copy, matching rccl_wrap.cc and the other staged
sources, and drop the direct entry.

Co-authored-by: Cursor <cursoragent@cursor.com>
@dlamd1dai
dlamd1dai requested review from a team October 18, 2026 20:11
@therock-pr-bot

Copy link
Copy Markdown

✅ 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?

@amd-argus amd-argus Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARGUS

No findings. The panel reviewed the changed files it was given and flagged nothing above Nit
level.

Bottom line: LGTM.

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

@alex-breslow-amd alex-breslow-amd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alex-breslow-amd
alex-breslow-amd merged commit 1a455a9 into develop Oct 20, 2026
74 of 78 checks passed
@alex-breslow-amd
alex-breslow-amd deleted the users/dondai/rccl-topo-expl-dup-arch-thresholds branch October 20, 2026 18:20
@dlamd1dai dlamd1dai self-assigned this Oct 21, 2026
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants