Fix OpenMP and link tools against hip::host#1352
Conversation
marbre
requested review from
YvanMokwinski,
jsandham,
kliegeois and
ntrost57
as code owners
| set_target_properties( ${lib_target_} PROPERTIES | ||
| INSTALL_RPATH "$ORIGIN/../llvm/${openmp_LIB_DIR}" | ||
| ) | ||
| if(NOT WIN32) |
There was a problem hiding this comment.
The if(NOT WIN32) and if(WIN32) changes should probably be propagated to rocblas etc as they do the same thing I think. Doesn't have to be done here, just mentioning it so other reviewers might see.
There was a problem hiding this comment.
rocBLAS will switch to next-cmake rather sooner than later which re-implements the current build logic. I am confident they have taken care and we didn't had similar build issues even with the old config so far.
| if (TARGET OpenMP::OpenMP_CXX) | ||
| set(ROCSPARSE_CLIENT_LIBS "OpenMP::OpenMP_CXX") | ||
| if(OPENMP_FOUND) | ||
| set(ROCSPARSE_CLIENT_LIBS "libomp") |
There was a problem hiding this comment.
Do we know why this doesn't seem to be required in rocblas?
There was a problem hiding this comment.
I haven't looked closely to the logic applied in rocBLAS and rather compared with the changes that broke the Windows build. Before that problematic patch, Windows was linking to libomp if OpenMP was found which I re-introduced here.
|
Previous Windows CI failure seems unrelated. |
Fix OpenMP and link tools against hip::host Building rocSPARSE was broken with commit 1171304 as it is links amd-llvm's OpenMP which is not available on Windows instead of libomp. Furthermore, re-adds to link `hip::host`. Fixes #1173. Co-authored-by: David Dixon <david.dixon@amd.com>
3 participants
Building rocSPARSE was broken with commit 1171304 as it is links amd-llvm's OpenMP which is not available on Windows instead of libomp. Furthermore, re-adds to link
hip::host.Fixes #1173.