Add rocWMMA to TheRock#1938
Conversation
44e4193 to
7bd48ac
Compare
bfb3c5f to
c2a5287
Compare
## Motivation These changes are required to integrate rocWMMA into TheRock. This PR subsumes the patches from my PR for TheRock here: ROCm/TheRock#1938 ## Technical Details Mostly changes to testing and CMake configuration. Notably: - Update googletest dependency. - Create new regression and smoke test ctest files. - Suggest using Ninja. - Export the project. ## Test Plan TheRock CI: https://github.com/ROCm/TheRock/actions/runs/18804601794 An equivalent PR from ROCm/rocWMMA passed all checks as well: https://github.com/ROCm/rocWMMA/pull/601/checks ## Test Result Tests passed ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
[rocWMMA] CMake changes for integration into TheRock ## Motivation These changes are required to integrate rocWMMA into TheRock. This PR subsumes the patches from my PR for TheRock here: ROCm/TheRock#1938 ## Technical Details Mostly changes to testing and CMake configuration. Notably: - Update googletest dependency. - Create new regression and smoke test ctest files. - Suggest using Ninja. - Export the project. ## Test Plan TheRock CI: https://github.com/ROCm/TheRock/actions/runs/18804601794 An equivalent PR from ROCm/rocWMMA passed all checks as well: https://github.com/ROCm/rocWMMA/pull/601/checks ## Test Result Tests passed ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
c2a5287 to
bd804f7
Compare
marbre
left a comment
There was a problem hiding this comment.
Please excuse the delayed review. Some comments.
Co-authored-by: Marius Brehler <marius.brehler@amd.com>
e7b6c39 to
867413d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds the rocWMMA library to TheRock, integrating it as a new math library component with comprehensive testing support on both Linux and Windows platforms.
- Integrates rocWMMA from rocm-libraries repository with CMake configuration
- Adds test infrastructure and CI configuration for rocWMMA validation
- Excludes rocWMMA from unsupported GPU targets (older gfx906, gfx101X, gfx103X families)
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
math-libs/artifact-rocwmma.toml |
Defines artifact packaging for rocWMMA library including headers, tests, and samples |
math-libs/CMakeLists.txt |
Adds rocWMMA subproject configuration with optional rocBLAS and benchmark dependencies |
examples/cpp-sdk-user/CMakeLists.txt |
Adds find_package configuration for rocWMMA in SDK user examples |
examples/clean_configure_test_project.cmake |
Propagates THEROCK_ENABLE_ROCWMMA flag to test project configuration |
examples/CMakeLists.txt |
Passes THEROCK_ENABLE_ROCWMMA flag to test configuration |
docs/development/windows_support.md |
Documents rocWMMA Windows support status |
docs/development/installing_artifacts.md |
Adds --rocwmma flag documentation for artifact installation |
cmake/therock_amdgpu_targets.cmake |
Excludes rocWMMA from unsupported GPU targets (gfx906, gfx101X, gfx103X, gfx1103) |
build_tools/install_rocm_from_artifacts.py |
Implements --rocwmma flag for artifact retrieval script |
build_tools/github_actions/test_executable_scripts/test_rocwmma.py |
Adds test execution script with sharding support for rocWMMA tests |
build_tools/github_actions/fetch_test_configurations.py |
Configures CI test job for rocWMMA with 4 shards on Linux and Windows |
README.md |
Documents THEROCK_ENABLE_ROCWMMA configuration flag |
CMakeLists.txt |
Defines rocWMMA feature with optional rocBLAS dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
stellaraccident
left a comment
There was a problem hiding this comment.
Generally looks good. I will review in detail after meetings today.
marbre
left a comment
There was a problem hiding this comment.
LGTM, assuming that the newly added rocWMMA tests all pass (as they already did on Windows). Let's wait for the CI signal prior to merging.
|
This caused build failures on the gfx101X and gfx103X GPU families on both Linux and Windows: #2127 Looking at the code in rocWMMA at https://github.com/ROCm/rocm-libraries/blob/07b9a2d3c3c7b3642bd41ec07a4096e877c7b729/projects/rocwmma/library/include/rocwmma/internal/config.hpp#L29-L80, unsupported architectures are compilation errors. At a minimum for now I think we'll need to exclude rocWMMA from certain target families in https://github.com/ROCm/TheRock/blob/main/cmake/therock_amdgpu_targets.cmake, but ideally the project would at least build in some form (fallback behavior, etc.) for all architectures. |
I am hesitant to do this because rocWMMA facilitates utilization of hardware constructs that are not available on the gfx10** devices, so it doesn't make much sense to provide support for them.
|
|
I opened #1944 to cover this. The only device we don't support but ideally should support is gfx1103. |
* DGGML_HIP_FORCE_ROCWMMA_FATTN_GFX12 is not needed anymore * More updates for README and manual_instructions * Add gfx1150 to the build-llamacpp-rocm worflow * Enabling rocWMMA_FATTN builds since rocWMMA headers are part of TheRock after ROCm/TheRock#1938 * rocWMMA patch is not needed anymore
## Motivation This PR adds the rocWMMA library to TheRock with testing. ## Technical Details We pull from rocm-libraries, and enable support and testing for Windows build. ## Test Plan Pull-Request CI run. ## Test Result Should check CI run when it completes. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Marius Brehler <marius.brehler@amd.com>
## Motivation These changes are required to integrate rocWMMA into TheRock. This PR subsumes the patches from my PR for TheRock here: ROCm/TheRock#1938 ## Technical Details Mostly changes to testing and CMake configuration. Notably: - Update googletest dependency. - Create new regression and smoke test ctest files. - Suggest using Ninja. - Export the project. ## Test Plan TheRock CI: https://github.com/ROCm/TheRock/actions/runs/18804601794 An equivalent PR from ROCm/rocWMMA passed all checks as well: https://github.com/ROCm/rocWMMA/pull/601/checks ## Test Result Tests passed ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Motivation
This PR adds the rocWMMA library to TheRock with testing.
Technical Details
We pull from rocm-libraries, and enable support and testing for Windows build.
Test Plan
Pull-Request CI run.
Test Result
Should check CI run when it completes.
Submission Checklist