Skip to content

[hipblaslt] Adjust HIPBLASLT_TENSILE_LIBRARY_DIR on Windows. - #522

Merged
stellaraccident merged 3 commits into
developfrom
users/scotttodd/tensile-windows-library-dir-1
Jul 21, 2025
Merged

stellaraccident merged 3 commits into
developfrom
users/scotttodd/tensile-windows-library-dir-1

Conversation

@ScottTodd

@ScottTodd ScottTodd commented Jul 7, 2025 •

Copy link
Copy Markdown
Member

This gets hipBLASLt tests passing on Windows in TheRock (see ROCm/TheRock#783), for ROCm/TheRock#681. It also enables PyTorch builds on Windows to use hipBLASLt without needing to manually copy folders around in the install directory.

This directory structure is now consistent with rocBLAS:

if( BUILD_WITH_TENSILE )
if (WIN32)
set( ROCBLAS_TENSILE_LIBRARY_DIR "\${CPACK_PACKAGING_INSTALL_PREFIX}/bin/rocblas" CACHE PATH "path to tensile library" )
else()
set( ROCBLAS_TENSILE_LIBRARY_DIR "\${CPACK_PACKAGING_INSTALL_PREFIX}${CMAKE_INSTALL_LIBDIR}/rocblas" CACHE PATH "path to tensile library" )
endif()

Splitting the structure leads to further complexity in all downstream projects and tooling.

This also gets hipblaslt tests passing on Windows.
@ScottTodd
ScottTodd requested a review from a team as a code owner July 7, 2025 23:34
stanleytsang-amd pushed a commit to stanleytsang-amd/rocm-libraries that referenced this pull request Jul 8, 2025
* decreasing number of synchronization
* Fixing wrong type, which combined with being uninitialized to zero led to random dirt
davidd-amd
davidd-amd previously approved these changes Jul 8, 2025

@davidd-amd davidd-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.

We will need to reboot tests

@ScottTodd

Copy link
Copy Markdown
Member Author

We will need to reboot tests

Yep. @geomin12 the "TheRock CI" workflows were skipped, so required checks there won't be satisfied until that part of #479 is resolved. We should also get hipblaslt and other subprojects added to here:

"""
This dictionary is used to map specific file directory changes to the corresponding build flag and tests
"""
subtree_to_project_map = {
"projects/rocprim": "prim",
"projects/rocthrust": "prim",
"projects/hipcub": "prim"
}
project_map = {
"prim": {
"cmake_options": "-DTHEROCK_ENABLE_PRIM=ON -DTHEROCK_ENABLE_ALL=OFF",
"project_to_test": "rocprim, rocthrust, hipcub",
"subtree_checkout": "projects/rocprim\nprojects/hipcub\nprojects/rocthrust",
},
}

Not sure about the "mci/rocm-libraries/" workflow, at least from a public developer perspective... I can look at the details and ask around internally with my AMD account though.

@ScottTodd

Copy link
Copy Markdown
Member Author

Not sure about the "mci/rocm-libraries/" workflow, at least from a public developer perspective... I can look at the details and ask around internally with my AMD account though.

I followed the link and clicked "rebuild".

@geomin12

geomin12 commented Jul 8, 2025

Copy link
Copy Markdown
Collaborator

We will need to reboot tests

Yep. @geomin12 the "TheRock CI" workflows were skipped, so required checks there won't be satisfied until that part of #479 is resolved. We should also get hipblaslt and other subprojects added to here:

"""
This dictionary is used to map specific file directory changes to the corresponding build flag and tests
"""
subtree_to_project_map = {
"projects/rocprim": "prim",
"projects/rocthrust": "prim",
"projects/hipcub": "prim"
}
project_map = {
"prim": {
"cmake_options": "-DTHEROCK_ENABLE_PRIM=ON -DTHEROCK_ENABLE_ALL=OFF",
"project_to_test": "rocprim, rocthrust, hipcub",
"subtree_checkout": "projects/rocprim\nprojects/hipcub\nprojects/rocthrust",
},
}

Not sure about the "mci/rocm-libraries/" workflow, at least from a public developer perspective... I can look at the details and ask around internally with my AMD account though.

yes, I plan on adding those cmake config fairly soon and getting those workflows updated

@ScottTodd
ScottTodd marked this pull request as draft July 8, 2025 23:26
@ScottTodd ScottTodd changed the title [hipblaslt] Unify HIPBLASLT_TENSILE_LIBRARY_DIR across platforms. [hipblaslt] Adjust HIPBLASLT_TENSILE_LIBRARY_DIR on Windows. Jul 9, 2025
@ScottTodd
ScottTodd marked this pull request as ready for review July 9, 2025 19:22
@ScottTodd
ScottTodd requested review from TorreZuk and davidd-amd July 9, 2025 19:22
Comment on lines +241 to 246
# Put files used at runtime adjacent to '.dll's and '.exe's in bin/.
set( HIPBLASLT_TENSILE_LIBRARY_DIR "\${CPACK_PACKAGING_INSTALL_PREFIX}${CMAKE_INSTALL_BINDIR}/hipblaslt" CACHE PATH "path to tensile library" )
else()
# Put files used at runtime adjacent to shared libraries ('.so') in
# lib/, separate from executables (no extension) in bin/.
set( HIPBLASLT_TENSILE_LIBRARY_DIR "\${CPACK_PACKAGING_INSTALL_PREFIX}${CMAKE_INSTALL_LIBDIR}/hipblaslt" CACHE PATH "path to tensile library" )

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.

Looks good to me if your windows tests pass.

You could fix the string "path to tensile library" to be "path to tensileLite library" (or whatever name should be the standard naming for tensile lite) but that is fine to clarify in separate PR as widespread documentation needs to be changed to avoid confusion. We shouldn't have two different libraries both referred to as tensile.

Would have been nice to have the math-CI windows pipelines running as well as a reference point, as PRs were made by multiple groups but never merged.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ellosel (or other hipblaslt developers) any recommendations for names to use or other text to include in these comments?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also yes, we'll get all the relevant CI pipelines hooked up so these sorts of changes can be tested by automation.

I'm juggling a bunch of patches but once I land them all I think we'll have CI-produced PyTorch wheels on Windows that can pass sanity checks. I built with all my patches except this one today and got this far:

>>> import torch
>>> batch_matrix1 = torch.ones(10, 2, 3, device="cuda")
>>> batch_matrix2 = torch.ones(10, 3, 4, device="cuda")
>>> result = torch.bmm(batch_matrix1, batch_matrix2)

rocBLAS error: Cannot read D:\projects\TheRock\external-builds\pytorch\.venv\Lib\site-packages\_rocm_sdk_libraries_gfx110X_dgpu\bin\/rocblas/library/TensileLibrary.dat: No such file or directory for GPU arch : gfx1100
 List of available TensileLibrary Files :

rocBLAS error: Could not initialize Tensile host:
directory_iterator::directory_iterator: The system cannot find the path specified.: "D:\projects\TheRock\external-builds\pytorch\.venv\Lib\site-packages\_rocm_sdk_libraries_gfx110X_dgpu\bin\/rocblas/library"

Once the patches are landed and some remaining CI pipelines are wired up we'll have those sorts of PyTorch tests running on ~every commit, for both Linux and Windows.

ScottTodd added a commit to ROCm/TheRock that referenced this pull request Jul 9, 2025
Tentative fix for #681. Related to
#792. Also enabled hipBLASLt tests
on our Windows CI, so progress on
#544.

This brings the hipBLASLt Tensile library directory structure in line
with what rocBLAS uses on Windows.

I've started sending upstream patches for review too:

* ROCm/rocm-libraries#522
* (abandoned?) ROCm/rocm-libraries#523
ammallya pushed a commit that referenced this pull request Jul 14, 2025
* decreasing number of synchronization
* Fixing wrong type, which combined with being uninitialized to zero led to random dirt

[ROCm/rocSPARSEcommit: ddd2d0a]
ammallya pushed a commit that referenced this pull request Jul 14, 2025
* decreasing number of synchronization
* Fixing wrong type, which combined with being uninitialized to zero led to random dirt

[ROCm/rocSPARSE commit: ddd2d0a]
@ScottTodd

Copy link
Copy Markdown
Member Author

All workflow runs completed. Can someone from CODEOWNERS please review this?

@ScottTodd

Copy link
Copy Markdown
Member Author

Ping? Still waiting on a code owner review.

@stellaraccident stellaraccident 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.

Fine for now. In the future, we want consistent directory layouts for kernel libraries across the operating systems (i.e. kernel libraries in bin is a bug). But fine to make this consistent for the moment.

@stellaraccident
stellaraccident merged commit 270ab3c into develop Jul 21, 2025
@stellaraccident
stellaraccident deleted the users/scotttodd/tensile-windows-library-dir-1 branch July 21, 2025 16:20
assistant-librarian Bot pushed a commit to ROCm/hipBLASLt that referenced this pull request Jul 21, 2025
[hipblaslt] Adjust HIPBLASLT_TENSILE_LIBRARY_DIR on Windows. (#522)

This gets hipBLASLt tests passing on Windows in TheRock (see
ROCm/TheRock#783), for
ROCm/TheRock#681. It also enables PyTorch
builds on Windows to use hipBLASLt without needing to manually copy
folders around in the install directory.

This directory structure is now consistent with rocBLAS:
https://github.com/ROCm/rocm-libraries/blob/c7fe831061a4cdc3a14e1aacab8c0738c21daf32/projects/rocblas/library/src/CMakeLists.txt#L825-L830
ammallya pushed a commit that referenced this pull request Jul 22, 2025
Bumps [rocm-docs-core](https://github.com/ROCm/rocm-docs-core) from 1.7.2 to 1.8.0.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.7.2...v1.8.0)

---
updated-dependencies:
- dependency-name: rocm-docs-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

[ROCm/hipSPARSE commit: 9e2262e]
ammallya pushed a commit that referenced this pull request Oct 27, 2025
Update target filtering for mma
ammallya pushed a commit that referenced this pull request Oct 28, 2025
Update target filtering for mma

[ROCm/rocwmma commit: 3449fb4]
mpanoop pushed a commit to mpanoop/rocm-libraries that referenced this pull request Oct 5, 2026
Bumps [rocm-docs-core](https://github.com/ROCm/rocm-docs-core) from 1.15.0 to 1.17.0.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.15.0...v1.17.0)

---
updated-dependencies:
- dependency-name: rocm-docs-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

[ROCm/rocThrust commit: bcf8583]
mpanoop pushed a commit to mpanoop/rocm-libraries that referenced this pull request Oct 5, 2026
Bumps [rocm-docs-core](https://github.com/ROCm/rocm-docs-core) from 1.15.0 to 1.17.0.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.15.0...v1.17.0)

---
updated-dependencies:
- dependency-name: rocm-docs-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

[ROCm/rocThrust commit: bcf8583]
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.

5 participants