[AIMIGRAPHX-1186] TheRock packaging: amdrocm-migraphx naming + drop weak deps (per-arch packaging in UFB)#5086
Open
kentqian wants to merge 5 commits into
Open
[AIMIGRAPHX-1186] TheRock packaging: amdrocm-migraphx naming + drop weak deps (per-arch packaging in UFB)#5086kentqian wants to merge 5 commits into
kentqian wants to merge 5 commits into
Conversation
…ng weak deps(recommend/suggest) to hard deps
…packaging (rfc0009)
kentqian
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates the TheRock packaging path in CMakeLists.txt (MIGRAPHX_PACKAGE_BACKEND=therock) to shift per-GPU dependency selection to per-arch meta packages with hard dependencies, and aligns the TheRock package naming with the amdrocm-* ecosystem (RFC0009). The default (non-TheRock) ROCm packaging path is intended to remain functionally unchanged.
Changes:
- Switch TheRock package naming from
MIGraphXtoamdrocm-migraphxand plumb the package name intorocm_create_package(NAME ...). - For per-arch mode, create per-arch meta package components (e.g.,
gfx90a) plus anallmeta component, installing a small README into each component’s doc dir. - Retain the existing device-all dependency behavior when
MIGRAPHX_THEROCK_GPU_ARCHis empty.
Comment on lines
+475
to
+477
| install(FILES ${_MGX_META_DOC} | ||
| DESTINATION share/doc/${_MGX_PKG_NAME}-${mgx_gpu_arch} | ||
| COMPONENT ${mgx_gpu_arch}) |
Comment on lines
+485
to
+487
| install(FILES ${_MGX_META_DOC} | ||
| DESTINATION share/doc/${_MGX_PKG_NAME}-all | ||
| COMPONENT all) |
Comment on lines
+502
to
+503
| # Legacy ROCm packaging. | ||
| set(_MGX_PKG_NAME "MIGraphX") |
Regressions detected 🔴 * No develop baseline was found for this PR's branch point; compared against the latest available develop run instead. |
|
…move per-arch to UFB
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5086 +/- ##
===========================================
- Coverage 92.90% 92.90% -0.00%
===========================================
Files 603 603
Lines 32526 32528 +2
===========================================
+ Hits 30217 30218 +1
- Misses 2309 2310 +1 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
CMakeLists.txt:475
- The else-branch comment was changed to "Legacy ROCm packaging", but this is still the default/traditional packaging backend. Calling it "legacy" is potentially misleading in configuration output and future maintenance.
# Legacy ROCm packaging.
set(_MGX_PKG_NAME "MIGraphX")
Comment on lines
451
to
454
| set(MIGRAPHX_THEROCK_GPU_ARCH "" CACHE STRING | ||
| "TheRock GPU arch(es) for per-GPU package dependencies. \ | ||
| Semicolon-separated list (e.g. gfx90a;gfx942;gfx950) for per-GPU deps, \ | ||
| or empty for device-all meta-package deps.") |
Comment on lines
+457
to
+461
| # Self-contained per-arch packages (amdrocm-migraphx-gfxNNN) and their per-GPU | ||
| # amdrocm-* deps are produced by the UFB Dockerfile cpack loop: one fat build, | ||
| # repackaged once per arch. Here we set only the base name + arch-neutral | ||
| # runtime dep; the device-all deps below are the standalone default and are | ||
| # overridden per-arch (amdrocm-dnn/blas-gfxNNN) at cpack time. |
kentqian
changed the title
Comment on lines
+453
to
+457
| # Only arch-neutral runtime deps are declared here (amdrocm-runtime), since | ||
| # that is the only component published as an arch-neutral package. | ||
| # The device libraries (MIOpen/rocBLAS) exist ONLY as per-arch packages | ||
| # (amdrocm-dnn-gfxNNN / amdrocm-blas-gfxNNN) with no arch-neutral variant, | ||
| # so they are intentionally omitted here. To produce a package with device |
eddieliao
approved these changes
Jul 24, 2026
causten
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Technical Details
One fat compile with multi-archs; UFB re-runs cpack once per GPU arch →
amdrocm-migraphx-gfxNNN, each hard-depending on that arch'samdrocm-dnn/blas-gfxNNN+amdrocm-runtime.Compatibility / scope
Arch-specific dependencies (moved out of CMake)
amdrocm-dnn-gfxNNN/amdrocm-blas-gfxNNN) with no arch-neutral variant, so CMake now declares only the arch-neutralamdrocm-runtimedependency.amdrocm-dnn-<arch>,amdrocm-blas-<arch>).Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot ApplicableFollow the LLVM AI Tool Use Policy for contributions using AI.