Repository navigation
Conversation
github-actions
Bot
added
project: hipblaslt
project: hipsparselt
ci:hipsparselt-fast
labels
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (48.80%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #12372 +/- ##
===========================================
+ Coverage 70.24% 70.99% +0.74%
===========================================
Files 2933 2935 +2
Lines 482160 483201 +1041
Branches 69508 69696 +188
===========================================
+ Hits 338677 343008 +4331
+ Misses 119197 116278 -2919
+ Partials 24286 23915 -371
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
|
@talumbau just to confirm that you would like a review of this PR? Asking as it is currently Draft |
talumbau
marked this pull request as ready for review
September 23, 2026 23:12
libtensilelite-host.so was installed into hipblaslt's runtime component, so the only way for hipSPARSELt to obtain it at run time is to depend on the whole hipblaslt package. Measured on a 7.0.2 drop build that package is 1600.6 MB, of which 466.5 MB is Tensile kernel data hipSPARSELt never uses, to supply one 6.5 MB shared library. Move the LIBRARY/RUNTIME artefacts of tensilelite-host into a new "tensilelite" CPack component, named tensilelite-host via LIBRARY_NAME + PACKAGE_NAME. The static archive and the public headers stay in devel: they are build-time only and hipsparselt-devel already depends on hipblaslt-devel. The component is declared only when tensilelite-host is actually a shared library. The check reads the target TYPE property rather than BUILD_SHARED_LIBS, because that variable is assigned later in this file (~line 805) and is therefore not reliable at the packaging scope. Refs AIHPBLAS-4434, AIHPBLAS-3522. Co-Authored-By: Claude <noreply@anthropic.com>
libhipblaslt.so carries DT_NEEDED on libtensilelite-host.so.1, so the
hipblaslt runtime package must say so now that the library ships separately.
Three things are needed together; splitting them leaves the package in a
state that is worse than before.
1. Publish the soname. rocm-cmake's packaging defaults set
CPACK_RPM_PACKAGE_AUTOREQPROV to Off for every ROCm library, so rpmbuild's
ELF scanning never runs and nothing Provides or Requires a soname today.
AIHPBLAS-4434 assumed re-enabling it would be a repo-wide change; CPack in
fact supports per-component overrides (CPackRPM.cmake:1098-1106 falls back
from CPACK_RPM_<COMPONENT>_PACKAGE_<TAG> to the global tag), so it can be
scoped to the tensilelite component alone. tensilelite-host then Provides
libtensilelite-host.so.1()(64bit), derived from the binary rather than
hand-written, so the capability tracks SOVERSION automatically.
2. Pin the version. hipblaslt, hipsparselt and tensilelite-host are intended
to move together within one ROCm release, so the dependency is an exact
version rather than the >= floor used for hipblas-common.
CPACK_PACKAGE_VERSION is computed inside rocm_create_package
(ROCMCreatePackage.cmake:281,296) from PROJECT_VERSION_* plus
ROCM_LIBPATCH_VERSION, and rocm_package_add_*_dependencies must be called
before rocm_create_package, so the same value is recomputed here.
3. Preserve the global dependency list. CPack uses
CPACK_RPM_<COMPONENT>_PACKAGE_REQUIRES *instead of* the global
CPACK_RPM_PACKAGE_REQUIRES once the component variable exists, so adding
any runtime-scoped dependency silently drops the global ones. Without this
the hipblaslt package loses "roctracer >= 1.0.0" and "rocm-core"; measured
on a gfx942 build its runtime Requires became just /bin/sh plus the soname.
rocm-core is added explicitly because rocm_package_add_rocm_core_dependency
appends it from inside rocm_create_package, after this point.
Resulting hipblaslt runtime Requires:
libtensilelite-host.so.1()(64bit)
rocm-core
roctracer >= 1.0.0
tensilelite-host = 1.5.0.70002
The pin is on VERSION, not VERSION-RELEASE: two builds from different commits
share a VERSION and differ only in RELEASE, so this gives release lockstep,
not ABI safety. Only SOVERSION discipline (AIHPBLAS-3522 decision D7) gives
the latter.
Refs AIHPBLAS-4434.
Co-Authored-By: Claude <noreply@anthropic.com>
talumbau
force-pushed
the
tja/tlh-r2-tensilelite-pkg
branch
from
September 23, 2026 23:22
94d7eae to
91ba6b0
Compare
davidd-amd
reviewed
Sep 23, 2026
Comment on lines
+195
to
+201
| # The shared library ships in its own "tensilelite" component (package | ||
| # tensilelite-host) rather than hipblaslt's runtime component. hipSPARSELt | ||
| # needs this one ~6.5 MB .so; bundling it in hipblaslt's runtime package | ||
| # would force every hipSPARSELt install to pull ~1.6 GB of Tensile kernel | ||
| # binaries it does not use. See AIHPBLAS-4434. | ||
| # The static archive and headers stay in devel: they are build-time only, | ||
| # and hipsparselt-devel already depends on hipblaslt-devel. |
Contributor
There was a problem hiding this comment.
Please remove all of the generated comments unless they are essential. We aren't doing anything here that is atypical for cmake.
davidd-amd
reviewed
Sep 23, 2026
| # Guard on the target TYPE rather than BUILD_SHARED_LIBS: that variable is not | ||
| # reliably set at this scope (it is assigned further down, near line 805), so | ||
| # testing it here would silently skip the component in a shared build. | ||
| if(TARGET tensilelite-host) |
Contributor
There was a problem hiding this comment.
This is fragile and non-deterministic. Prefer to gate on a config option.
davidd-amd
reviewed
Sep 23, 2026
| rocm_package_add_rpm_dependencies(COMPONENT runtime | ||
| DEPENDS "${CPACK_RPM_PACKAGE_REQUIRES}") | ||
| endif() | ||
| if(ROCM_DEP_ROCMCORE) |
Contributor
There was a problem hiding this comment.
Where is ROCM_DEP_ROCMCORE introduced?
davidd-amd
reviewed
Sep 23, 2026
Comment on lines
+954
to
+955
| rocm_package_add_rpm_dependencies(COMPONENT runtime | ||
| DEPENDS "libtensilelite-host.so.1()(64bit)") |
Contributor
There was a problem hiding this comment.
Are we hardcoding a .1 here?
newling
reviewed
Sep 25, 2026
newling
left a comment
newling
left a comment
Contributor
There was a problem hiding this comment.
The "Why" makes sense to me. One thing we might consider in the future reconsidering the directory structure around hipblaslt and tensilelite, as tensilelite is a standalone thing.
For the "What", with my blurry vision this looks fine to me too. To get into more into the debian/cmake details that I don't have a good intuition for, I got an agent review (see here). It references David's preceding review. It has an accompanying branch with commits corresponding to each suggestion. Feel free to cherry-pick / ignore.
Drop the tensilelite-host version pin; mark devel placement as interim hipblaslt's runtime package pinned tensilelite-host to hipBLASLt's exact package version. That records "tensilelite version equals hipBLASLt version" in package metadata, which a standalone tensilelite package would have to undo. It also rebuilt CPACK_PACKAGE_VERSION by hand because the real value is computed later inside rocm_create_package, so a change to rocm-cmake's formula would break the pin silently. Remove the pin and the version recomputation. RPM keeps the soname dependency libtensilelite-host.so.1()(64bit). DEB gets no soname dependency, so it now depends on tensilelite-host with no version. Reword the install comment so the archive, headers and imported target living in hipblaslt-devel read as interim, not as the intended layout. Fix the TYPE-guard comment: BUILD_SHARED_LIBS at that point holds HIPBLASLT_BUILD_SHARED_LIBS, which is independent of TENSILELITE_BUILD_SHARED_LIBS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 task
talumbau
added a commit
that referenced
this pull request
Oct 16, 2026
…12888) JIRA ID : AIHPBLAS-4434 ## Motivation hipSPARSELt uses the TensileLite host library at run time but does not need hipBLASLt's large kernel-data package. The existing split placed TensileLite files in components owned by hipBLASLt, which also forced those packages to use the hipBLASLt version. This change makes TensileLite an independently versioned CMake and packaging project while preserving hipBLASLt and hipSPARSELt consumption. ## Technical Details - Define the TensileLite project version with `TENSILELITE_PROJECT_VERSION`, initially `1.5.0`, and install `tensilelite-config.cmake` with the `roc::tensilelite-host` export. - Generate independent shared, static-development, and AddressSanitizer packages through `rocm_create_package`. Nested package components use distinct names so they cannot enter hipBLASLt packages. - Keep the existing `make package` entry point by running the nested TensileLite CPack configuration after hipBLASLt packaging. - Make hipBLASLt depend on the matching TensileLite runtime and development package versions. Keep hipSPARSELt's CMake dependency on hipBLASLt while changing its runtime package dependency to `tensilelite-host`. - Preserve bundled Origami as a separate static development package and retain upgrade metadata for files formerly owned by hipBLASLt packages. ## Related - Work tracking: AIHPBLAS-4434 - Related PR: #12372 ## Device / Architecture Coverage This changes CMake exports and package construction without changing kernels, dispatch, or device behavior. Host builds used `gfx942` as the configured target; automated GitHub build and packaging checks are pending. ## Test Plan - Build and package hipBLASLt with shared and static TensileLite configurations. - Build and package TensileLite directly as a standalone project. - Verify AddressSanitizer packaging contains the TensileLite shared library and license. - Configure and link consumers of the shared and static `roc::tensilelite-host` exports and the `roc::hipblaslt` export. - Exercise the TheRock dependency-provider flow through Origami, hipBLASLt, and hipSPARSELt. - Upgrade from the pre-split Debian packages and check package-manager consistency. ## Test Result - Shared hipBLASLt build passed; one CPack invocation produced `hipblaslt`, `hipblaslt-dev`, `tensilelite-host`, `tensilelite-host-dev`, and `origami-static-dev`. - Static TensileLite build passed and produced `tensilelite-host-static-dev` and `origami-static-dev` without empty runtime or development packages. - Standalone and AddressSanitizer builds and Debian packaging passed. The AddressSanitizer package contains `libtensilelite-host.so` and depends on `rocm-core-asan`. - Shared and static package consumers configured and linked successfully. - TheRock Origami, hipBLASLt, and hipSPARSELt configure, build, install, and artifact classification passed. - Debian upgrade testing completed with `apt` status 0 and a clean `dpkg --audit` result. - The local full TensileLite Python suite was attempted: 7,845 passed and 452 skipped; 50 failed and 22 errored because the host lacks `/usr/bin/amdclang++` and gfx1250 support. The failures do not touch the changed CMake packaging paths. - Automated GitHub checks are pending. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. ## Risk level Medium (3/5): this changes package ownership, dependency metadata, and installed CMake exports across TensileLite, hipBLASLt, Origami, and hipSPARSELt. It does not change runtime or device code. --------- Co-authored-by: T.J. Alumbaugh <talumbau@amd.com> Co-authored-by: Claude <noreply@anthropic.com>
This branch has not been deployed
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.
Why
hipSPARSELt needs exactly one thing out of the hipBLASLt build at run time: the ~6.5 MB shared library
libtensilelite-host.so. Today that library is installed into hipBLASLt'sruntimecomponent, so the only way to depend on it is to depend on the wholehipblasltpackage.Measured on a gfx942 drop build:
hipblasltpackagelibtensilelite-host.soThat is the cost AIHPBLAS-4434 was filed to avoid.
What
Two commits.
1. Ship the library in its own package. The
LIBRARY/RUNTIMEartefacts oftensilelite-hostmove into a newtensileliteCPack component, namedtensilelite-hostviaLIBRARY_NAME+PACKAGE_NAME. The static archive and public headers stay indevel— they are build-time only, andhipsparselt-develalready depends onhipblaslt-devel.The component is declared only when
tensilelite-hostis actually a shared library. The check reads the targetTYPEproperty rather thanBUILD_SHARED_LIBS, because that variable is assigned later in the same file (~line 805) and is not reliable at packaging scope.This also widens the
install-pkgglob intasks.py: hipBLASLt no longer produces onlyhipblaslt-*packages, and installing just that glob leaves the.soon the floor — a downstreamfind_package(hipblaslt)then fails on the importedroc::tensilelite-hosttarget pointing at a file that was never installed.2. Declare hipBLASLt's dependency on it. Three things that must land together:
CPACK_RPM_PACKAGE_AUTOREQPROVtoOfffor every ROCm library, so rpmbuild's ELF scan never runs and nothingProvides/Requiresa soname. AIHPBLAS-4434 assumed re-enabling it would be a repo-wide change; CPack in fact supports per-component overrides (CPackRPM.cmake:1098-1106), so it is scoped to the new component alone.tensilelite-hostthenProvides libtensilelite-host.so.1()(64bit), derived from the binary rather than hand-written, so the capability tracks SOVERSION automatically.>=floor used forhipblas-common.CPACK_RPM_<COMPONENT>_PACKAGE_REQUIRESinstead of the globalCPACK_RPM_PACKAGE_REQUIRESonce the component variable exists. Without this, adding any runtime-scoped dependency silently dropsroctracer >= 1.0.0androcm-corefrom the hipblaslt package.Result
tensilelite-hostis 1.2 MB.TheRock impact: none
CPack/RPM-only. TheRock claims artifacts by path glob from a
cmake --installtree (math-libs/BLAS/artifact-blas.toml), not by CPack component. A plaincmake --installemits all components and the installed path of the.sois unchanged.Known limitation
The pin is on VERSION, not VERSION-RELEASE. Two builds from different commits share a VERSION and differ only in RELEASE, so this gives release lockstep, not ABI safety. Only SOVERSION discipline (AIHPBLAS-3522 decision D7) gives the latter.
Testing
gfx942, ROCm 7.0.2, RHEL 9.5.
hipblaslt-test --gtest_filter=*smoke*: 3,118 passed.lddreports no missing libraries. Full install-scenario matrix in the linked plan.Refs AIHPBLAS-4434, AIHPBLAS-3522.