Build system improvements #1597
Draft
+81
−87
Conversation
This file contains 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
ellosel
commented
Jan 27, 2025
| @@ -26,7 +26,7 @@ function(virtualenv_create) | |||
| endfunction() | |||
|
|
|||
| function(virtualenv_install) | |||
| virtualenv_create() | |||
| virtualenv_create() | |||
powderluv
reviewed
Feb 2, 2025
| @@ -82,7 +82,7 @@ include(cmake/Dependencies.cmake) | |||
|
|
|||
| # Append our library helper cmake path and the cmake path for hip (for convenience) | |||
| # Users may override HIP path by specifying their own in CMAKE_MODULE_PATH | |||
| list( APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${ROCM_PATH}/lib/cmake/hip /opt/rocm/lib/cmake/hip ${HIP_PATH}/cmake ) | |||
| list( APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${ROCM_PATH}/lib/cmake/hip /opt/rocm/lib/cmake/hip ${HIP_PATH}/cmake ${CMAKE_CURRENT_SOURCE_DIR}/tensilelite/Tensile/cmake) | |||
There was a problem hiding this comment.
lets not include /opt/rocm anywhere. It will cause hard fails but it is easy to fix.
There was a problem hiding this comment.
In what way does it cause a hard failure?
| @@ -153,12 +153,11 @@ if (LINK_BLIS) | |||
| endif() | |||
|
|
|||
| if (USE_CUDA) | |||
| find_package( CUDA REQUIRED ) | |||
| find_package( CUDA REQUIRED ) | |||
solaslin
added
the
noCI
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.
The hipblaslt build system was inherited from rocblas. However, the physical design of the project is different. In particular, tensile is embedded in hipblalt, rather than being a separate project. Nevertheless, all of the machinery to install tensile into a python virtual environment is still in the hipblaslt build system. This has several consequences:
makeis unable to detect that anything changes.This PR reconciles these issues by taking initial steps to properly include tensile into the hipblaslt build system. To begin, Tensile is no longer pip installed through the
execute_processAPI. Rather, we use andadd_subdirectorycall which ensures that source file changes in Tensile are observable and executingmakein the build directory will retrigger a rebuild of the necessary targets. Further,ALLis removed from the custom target for generating the gemm libraries withTensileCrateLibraryso that they are no longer unconditionally rebuilt every timemakeis executed. These changes will significantly reduce build times for developers that are only working on the cpp interfaces in hipblaslt.In addition, we made some changes to source files to resolve compiler warnings.
We will include documentation updates for building hipblaslt.