Skip to content

[AIMIGRAPHX-1057] Fix TheRock multi-arch packaging: use Recommends for per-GPU dependencies#4916

Open
kentqian wants to merge 7 commits into
developfrom
kqian/therock/cmake/multi-arch
Open

[AIMIGRAPHX-1057] Fix TheRock multi-arch packaging: use Recommends for per-GPU dependencies#4916
kentqian wants to merge 7 commits into
developfrom
kqian/therock/cmake/multi-arch

Conversation

@kentqian
Copy link
Copy Markdown
Contributor

Motivation

Multi-arch fat binary .deb packages currently declare all per-GPU runtime dependencies (amdrocm-dnn, amdrocm-blas) as hard Depends. This makes the package uninstallable on any system that doesn't have all GPU arch libraries available — even if the user only has one GPU type.

Technical Details

When MIGRAPHX_THEROCK_GPU_ARCH contains multiple architectures (e.g. gfx94x;gfx950;gfx110x;gfx120x), move per-GPU amdrocm-dnn/amdrocm-blas dependencies from Depends to Recommends (deb) / Suggests (rpm). Only the libraries matching the GPU actually present are needed at runtime.

  • Single-arch builds are unchanged — per-GPU deps remain hard Depends.
  • Also adds a device-all fallback path when MIGRAPHX_THEROCK_GPU_ARCH is empty.
  • Removes the FATAL_ERROR guard so the therock backend can be used without specifying GPU arch.

Changelog Category

Add a CHANGELOG.md entry for any option other than Not Applicable

    • Added: New functionality.
    • Changed: Changes to existing functionality.
    • Removed: Functionality or support that has been removed. (Compared to a previous release)
    • Optimized: Component performance that has been optimized or improved.
    • Resolved Issues: Known issues from a previous version that have been resolved.
    • Not Applicable: This PR is not to be included in the changelog.

@kentqian kentqian requested review from kahmed10 and pfultz2 May 27, 2026 18:23
@kentqian kentqian self-assigned this May 27, 2026
@kentqian kentqian requested a review from causten as a code owner May 27, 2026 18:23
Copilot AI review requested due to automatic review settings May 27, 2026 18:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts TheRock packaging dependency metadata so multi-arch “fat binary” packages don’t become uninstallable by requiring all per-GPU runtime libraries as hard dependencies, while keeping single-arch behavior unchanged.

Changes:

  • Removes the hard requirement that MIGRAPHX_THEROCK_GPU_ARCH must be set when using MIGRAPHX_PACKAGE_BACKEND=therock.
  • For multi-arch builds, moves per-GPU amdrocm-dnn / amdrocm-blas dependencies from hard Depends into Recommends (deb) / Suggests (rpm).
  • Adds an empty-arch “device-all” path that depends on arch-independent meta packages.

Comment thread CMakeLists.txt Outdated
Comment on lines +444 to +448
# Multi-arch fat binary: per-GPU libs are Recommends (only the
# libs for the GPU actually present are needed at runtime).
string(REPLACE ";" ", " _MGX_GPU_DEPS_CSV "${_MGX_GPU_DEPS}")
set(CPACK_DEBIAN_RUNTIME_PACKAGE_RECOMMENDS "${_MGX_GPU_DEPS_CSV}")
set(CPACK_RPM_PACKAGE_SUGGESTS "${_MGX_GPU_DEPS_CSV}")
Comment thread CMakeLists.txt Outdated
Comment on lines +446 to +448
string(REPLACE ";" ", " _MGX_GPU_DEPS_CSV "${_MGX_GPU_DEPS}")
set(CPACK_DEBIAN_RUNTIME_PACKAGE_RECOMMENDS "${_MGX_GPU_DEPS_CSV}")
set(CPACK_RPM_PACKAGE_SUGGESTS "${_MGX_GPU_DEPS_CSV}")
@kentqian kentqian changed the title Fix TheRock multi-arch packaging: use Recommends for per-GPU dependencies [AIMIGRAPHX-1057] Fix TheRock multi-arch packaging: use Recommends for per-GPU dependencies May 27, 2026
@kentqian kentqian force-pushed the kqian/therock/cmake/multi-arch branch from b1e20e8 to b6b2096 Compare May 28, 2026 04:37
Comment thread CMakeLists.txt Outdated
@kentqian kentqian requested review from causten and kahmed10 May 30, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants