Skip to content

[windows] Patch TENSILE_LIBRARY_DIR in hipBLASLt. - #783

Merged
ScottTodd merged 14 commits into
mainfrom
users/scotttodd/windows-blas-library-dirs
Jul 9, 2025
Merged

ScottTodd merged 14 commits into
mainfrom
users/scotttodd/windows-blas-library-dirs

Conversation

@ScottTodd

@ScottTodd ScottTodd commented Jun 6, 2025 •

Copy link
Copy Markdown
Member

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:

Comment thread .github/workflows/test_hipblaslt.yml Resolved
Comment thread .github/workflows/test_hipblaslt.yml Outdated
Comment on lines -37 to -38
# Currently, this test is not working with Windows. This test will be enabled once this library has been enabled
if: ${{ inputs.platform == 'linux' }}

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.

Tests pass on Windows gfx1151: https://github.com/ROCm/TheRock/actions/runs/15495327375/job/43649536258?pr=783

[----------] Global test environment tear-down
[==========] 5766 tests from 2 test suites ran. (470772 ms total)
[  PASSED  ] 5766 tests.
hipBLASLt version: 100000
hipBLASLt git version: c630f202
command line: B:\actions-runner\_work\TheRock\TheRock\build\bin\hipblaslt-test.exe --gtest_filter=*pre_checkin* 

Odd checkout failures on rocBLAS and rocPRIM though:
https://github.com/ROCm/TheRock/actions/runs/15495327375/job/43649536566?pr=783

 Deleting the contents of 'B:\actions-runner\_work\TheRock\TheRock'
Error: File was unable to be removed Error: EPERM: operation not permitted, unlink 'B:\actions-runner\_work\TheRock\TheRock\build\bin\amd_comgr0605.dll'

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.

Failed again on a retry: https://github.com/ROCm/TheRock/actions/runs/15495327375/job/43740951661?pr=783#step:2:28

@amd-justchen can you debug on that runner (windows-strix-halo-gpu-rocm-1)?

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.

Sure! I'll take a look! I just about am done monitoring the CI CPU builders. That error looks like the same one before. I'll be able to remote in to the machine to troubleshoot.

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.

I suspect the build is only just now producing those files, so we should see if any permission/owner bits are set differently. Might need to change how those projects are built or make our CI workflows try harder to clean up the files during startup/shutdown.

A common post-job hook for a self-hosted runner deletes the source directory. We used these on IREE: https://github.com/iree-org/iree/tree/41dcee93c7157955d94973addf6770cecf926849/build_tools/github_actions/runner/config/hooks. See pre_job.sh, cleanup_workdir.sh, and https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/running-scripts-before-or-after-a-job.

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.

Argh: https://github.com/ROCm/TheRock/actions/runs/16148483933/job/45579902751

Still failing with "File was unable to be removed" for rocblas, hipblaslt, rocprim, and hipcub tests. Going to retry those to see if the runners are flaky somehow.

@amd-justchen

Copy link
Copy Markdown
Contributor

https://github.com/ROCm/TheRock/actions/runs/15495327375/job/43649536566?pr=783#step:2:32

Just noting this error on the selfhosted gpu test runner

Error: File was unable to be removed Error: EPERM: operation not permitted, unlink 'B:\actions-runner\_work\TheRock\TheRock\build\bin\rocblas.dll'

@amd-justchen amd-justchen 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

@geomin12

Copy link
Copy Markdown
Contributor

does tensile work with 1151? seems like the rocblas is failing due to no tensile .dat found for 1151 :(

@ScottTodd

Copy link
Copy Markdown
Member Author

does tensile work with 1151? seems like the rocblas is failing due to no tensile .dat found for 1151 :(

Perhaps not...

I'm having trouble debugging locally using the artifacts produced by CI though: https://gist.github.com/ScottTodd/bf9519793b0d99df5e1b89e0be479b89. Might need to build from source to debug further.

@ScottTodd

ScottTodd commented Jul 1, 2025 •

Copy link
Copy Markdown
Member Author

does tensile work with 1151? seems like the rocblas is failing due to no tensile .dat found for 1151 :(

Perhaps not...

I'm having trouble debugging locally using the artifacts produced by CI though: https://gist.github.com/ScottTodd/bf9519793b0d99df5e1b89e0be479b89. Might need to build from source to debug further.

I had the same issue with a source build on my primary gfx1100 dev machine so I pushed a patch that now lets me run the rocBLAS tests locally on Windows (they all passed). Next I should be able to pull the gfx1151 artifacts and test on gfx1151 dev machine (or build there, but that will take longer).

@ScottTodd

Copy link
Copy Markdown
Member Author

Okay I can reproduce the rocblas test failures locally on two machines (gfx1151 and gfx1100) now. Next step is to double check the directory layout of therock-dist compared against python build_tools/install_rocm_from_artifacts.py --run-id 16010675859 --amdgpu-family gfx110X-dgpu --output-dir %HOME%\.therock\install_16010675859 --blas --tests.

@ScottTodd

Copy link
Copy Markdown
Member Author

Ugh. I wrote this patch for the old CMake, we've since moved to next-cmake, which repeated the same pattern.

@ScottTodd

Copy link
Copy Markdown
Member Author

Validated more carefully on a local build, hopefully works again now...

@ScottTodd

Copy link
Copy Markdown
Member Author

CI failed again, and I can reproduce locally again... strange. Testing is harder now that we've had a significant compile time regression though: #978

@ScottTodd
ScottTodd marked this pull request as ready for review July 7, 2025 22:10
@ScottTodd

Copy link
Copy Markdown
Member Author

Okay, tests are passing again, including newly enabled hipblaslt tests on Windows. I'm going to need reviews from hipblaslt and rocblas maintainers for this one though, so I'll also prepare a PR in https://github.com/ROCm/rocm-libraries and may need to get the attention of some Windows HIP SDK maintainers.

@ScottTodd

Copy link
Copy Markdown
Member Author

Sent two upstream PRs so far:

I'll need at least one more for the rocblas_parse_data.cpp changes, since there are a few other files that have repeated that code pattern of running .py scripts directly instead of via python script.py on Windows. Those changes aren't load bearing for users, though they would help for library developers who want to run tests locally, like me.

@ScottTodd
ScottTodd requested a review from stellaraccident July 8, 2025 00:38
@ScottTodd

Copy link
Copy Markdown
Member Author

@ellosel want to review this PR while we work through the CI jobs in the rocm-libraries PRs? Once those land and get propagated to TheRock we can then drop these patches.

@ScottTodd

Copy link
Copy Markdown
Member Author

Test failures from files remaining open/locked are back, and discussion on the upstream PRs is converging on an alternate solution - keeping the lib/ and bin/ directory structure for rocblas and getting hipblaslt to match it as needed.

@ScottTodd
ScottTodd marked this pull request as draft July 8, 2025 23:32
@ScottTodd
ScottTodd removed the request for review from stellaraccident July 8, 2025 23:32
@ScottTodd ScottTodd changed the title [windows] Patch TENSILE_LIBRARY_DIR in rocBLAS and hipBLASLt. [windows] Patch TENSILE_LIBRARY_DIR in hipBLASLt. Jul 9, 2025
@ScottTodd
ScottTodd marked this pull request as ready for review July 9, 2025 19:24
@ScottTodd

Copy link
Copy Markdown
Member Author

Reworked the patches on this PR and the upstream PRs. PTAL.

@ScottTodd

Copy link
Copy Markdown
Member Author

Feedback is looking good on the upstream PR, going to merge this with the existing approval to start flushing my PR queue. Open to post-submit feedback and reverting if others have objections.

@ScottTodd
ScottTodd merged commit 36c10f3 into main Jul 9, 2025
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Jul 9, 2025
@ScottTodd
ScottTodd deleted the users/scotttodd/windows-blas-library-dirs branch July 9, 2025 22:23
ScottTodd added a commit that referenced this pull request Jul 10, 2025
Progress on #827 (!!)

Tested at https://github.com/ROCm/TheRock/actions/runs/16185309410,
which triggered these:

* https://github.com/ROCm/TheRock/actions/runs/16186383600 (gfx1151)
* https://github.com/ROCm/TheRock/actions/runs/16186941663 (gfx120X-all)
* https://github.com/ROCm/TheRock/actions/runs/16186966931
(gfx110X-dgpu)

Some next steps after this, roughly in order:

* Monitor scheduled jobs to check that wheels are successfully built and
uploaded
* Download/install and test the wheels on various systems, exercising
components like hipBLASLt
* Update support status:
  * https://github.com/ROCm/TheRock?tab=readme-ov-file#support-status
*
https://github.com/ROCm/TheRock/blob/main/RELEASES.md#python-packages-support-status
* Build torchvision and torchaudio (need some patches)
* Connect with other build/test/release workflows so we get pre-submit,
post-submit, and scheduled coverage for these builds/releases/tests
* Rebase patches onto PyTorch 2.8 (and/or HEAD), land patches upstream
* Add features like Triton support (aotriton?) for optimized flash
attention kernels, etc.
* Expand the list of supported gfx targets

---

~~This depends on some other open PRs, but I have tested parts of it all
the way through to dev wheel publishing and then installation and
testing on a dev machine:
https://github.com/ROCm/TheRock/actions/runs/16181411431/job/45678668536,
#827 (comment)

~~See the other PRs:~~

* ~~#783
* ~~#980
* ~~#988
* ~~#990
stellaraccident pushed a commit to ROCm/rocm-libraries that referenced this pull request Jul 21, 2025
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants