-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add LLVM_DEFAULT_TARGET_TRIPLE to find new location of -lomp #839
base: develop
Are you sure you want to change the base?
Add LLVM_DEFAULT_TARGET_TRIPLE to find new location of -lomp #839
Conversation
An upstream llvm change enables LLVM_ENABLE_PER_TARGET_RUNTIME_DIR by default for the openmp build. This installs the openmp libraries into /opt/rocm-ver/llvm/lib/x86_64-unknown-linux-gnu instead of /opt/rocm-ver/llvm/lib. Currenty, hipBLAS only looks in /lib. Prepend lib/x86_64-unknown-linux-gnu to -L and --rpath to avoid linker error.
if(LLVM_DEFAULT_TARGET_TRIPLE) | ||
list( APPEND COMMON_LINK_LIBS "-L\"${HIP_CLANG_ROOT}/lib/${LLVM_DEFAULT_TARGET_TRIPLE}\"") | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you have no way to add this into OpenMP::OpenMP_CXX as that would be cleaner approach. I thought we were moving to use /bin/amdclang as well. Otherwise appears fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The find_package(OpenMP)
is actually using FindOpenMP.cmake from the cmake install itself. From what I have seen, it is finding libgomp.so from a system package and not ROCm. Passing a custom CMAKE_PREFIX_PATH
will not work either for this as they turn off these search paths in FindOpenMP.cmake. This is probably why the -L /lib
had to be added in the first place. The openmp install does not have a config file to query either.
As far as using amdclang
, I can make that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, it is getting messy but acceptable I guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI looks happy with it, so fine by me.
Summary of proposed changes:
An upstream llvm change enables
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
by default for the openmpbuild. This installs the openmp libraries into
/opt/rocm-ver/llvm/lib/x86_64-unknown-linux-gnu
instead of/opt/rocm-ver/llvm/lib
. Currenty, hipBLAS only looks in /lib. Prepend lib/x86_64-unknown-linux-gnu to-L
and--rpath
to avoid a linker error when the upstream change lands inamd-staging
.