Enable building on Windows with necessary modifications #2167
Conversation
amd-mtrifuno
commented
Jun 3, 2025
amd-mtrifuno
commented
- Add script to download Windows dependencies
- Introduce rmake.py script for Windows builds
- Disable roctracer and rocroller - unsupported on Windows
- Update packaging configurations for Windows
- Include OpenBLAS library for Windows builds
- Adapt environment variable settings for Windows
- Implemented cross-platform libraries copying
- Fix linker issues related to platform differences
amd-mtrifuno
requested review from
jichangjichang,
KKyang,
vin-huang,
imcarsonliao,
hcman2,
Serge45,
Jinp800125,
TonyYHsieh,
solaslin and
aazz44ss
as code owners
There was a problem hiding this comment.
Fyi, we will shortly be deleting all of the default cmake files and switching to those in the next-cmake directory. The next-cmake build is tested on Windows via TheRock and likely shouldn't need any changes for basic Windows compat. The default cmake build has a number of portability problems that we do not want to address.
I'd recommend using next-cmake files for the time being until we get them switched.
Also, we can't accept changes upstream anymore which presume the hipsdk directory layout. If those are necessary, they are generally part of the hipsdk build, not part of the OSS build.
|
|
||
| target_link_libraries( hipblaslt-test PRIVATE ${COMMON_LINK_LIBS} ) | ||
|
|
||
| if (WIN32) |
There was a problem hiding this comment.
Sorry, we can't land changes like this anymore. It is never appropriate to be copying files from system32 or using a platform gate to move files around by absolute path. Getting these things lined up is a job for whatever is building this project, not just something to be done unconditionally here.
| @@ -78,6 +78,22 @@ endif() | |||
|
|
|||
| # Target compile definitions | |||
| if(NOT BUILD_CUDA) | |||
| if (WIN32) | |||
There was a problem hiding this comment.
No - I don't see why this is win32 guarded.
| @@ -207,6 +223,10 @@ set_target_properties(hipblaslt PROPERTIES CXX_VISIBILITY_PRESET "hidden" VISIBI | |||
| set_target_properties(hipblaslt PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/staging") | |||
| set_target_propertieS(hipblaslt PROPERTIES DEBUG_POSTFIX "-d") | |||
|
|
|||
| if (WIN32 AND BUILD_CLIENTS) | |||
There was a problem hiding this comment.
Cannot land things like this anymore. This is not win32 generally but related to the expectations of your build system.
| @@ -239,7 +259,7 @@ install( | |||
|
|
|||
| if ( NOT BUILD_CUDA ) | |||
| if (WIN32) | |||
| set( HIPBLASLT_TENSILE_LIBRARY_DIR "\${CPACK_PACKAGING_INSTALL_PREFIX}hipblaslt/bin" CACHE PATH "path to tensile library" ) | |||
| set( HIPBLASLT_TENSILE_LIBRARY_DIR "\${CPACK_PACKAGING_INSTALL_PREFIX}/bin/hipblaslt" CACHE PATH "path to tensile library" ) | |||
There was a problem hiding this comment.
Fyi - we're aligning these libraries to ask be based on the posix paths. The hipBLASLt host code has already been updated to use lib as the location for all platforms.
| COMMENT "Copying .co and .dat files" | ||
| ) | ||
|
|
||
| file(GLOB FILES_TO_COPY "${CMAKE_CURRENT_BINARY_DIR}/*.co" "${CMAKE_CURRENT_BINARY_DIR}/*.dat") |
There was a problem hiding this comment.
I don't understand how this can work: the glob will be evaluated at configure time but it is trying to find files that are generated at build time. I think this will only work after the first build, which would be wrong.
| @@ -0,0 +1,73 @@ | |||
| # ######################################################################## | |||
| # Copyright (C) 2022-2023 Advanced Micro Devices, Inc. | |||
There was a problem hiding this comment.
TheRock is ROCm's official windows build. If hipsdk needs a custom toolchain, then it is local to that, not global for Windows
