Drop clang rt builtins linking on hip::host#2219
Conversation
Ref this comment ROCm#2217 (comment): > Yea, this should only be linked on hip::device as I believe its needed > for __fp16 support on the GPU. If users need this on the host-side, > then they should explicitly add it.
|
Ping @pfultz2 |
|
One difficulty for users to link this library is that users may not know the location of this library since it is compiler implementation detail. Can we add a cmake flag to control it? Users can turn on/off the flag to link this library. |
|
By this library you mean the runtime lib? I think the standard way to use the runtime lib is through the compiler flag set_property(TARGET hip::device APPEND PROPERTY
INTERFACE_COMPILE_OPTIONS --rtlib=compiler-rt
) |
|
--rtlib=compiler-rt calls some unwinding functions which may not available be in the default unwind lib, which may require an additional option for using llvm unwind lib. Besides, we only want to use a few fp16 conversion functions in compiler-rt and do not want to use other functions in compiler-rt. Using --rtlib=compiler-rt will make us subject to other issues of compiler-rt. |
How about just adding a new target for the builtin lib then so that people can use that if they really need to? |
sounds good to me. thanks. |
|
Ok, done @yxsamliu. Could you also take a look at my comment here #2217 (comment) |
| endif() | ||
|
|
||
| add_library(hip::clang_rt_builtins INTERFACE) | ||
| target_link_libraries(hip::clang_rt_builtins PUBLIC -L${HIP_CLANG_INCLUDE_PATH}/../lib/linux -lclang_rt.builtins-x86_64) |
There was a problem hiding this comment.
This should use find_library to find clang_rt_builtins:
find_library(CLANG_RT_BUILTINS lang_rt.builtins-x86_64 HINTS ${HIP_CLANG_INCLUDE_PATH}/../lib/linux PATHS /opt/rocm/llvm)This way a user doesnt necessarily need to set HIP_CXX_COMPILER as it will still be found along the CMAKE_PREFIX_PATH.
There was a problem hiding this comment.
Yeah, much better. Let me do that tomorrow.
There was a problem hiding this comment.
Hm, unfortunately it's not really working @pfultz2, in the sense that it's only reasonable to assume the user / package manager passes the install prefix of LLVM, not the path to clang, and in that case the following does not work:
find_library(CLANG_RT_BUILTINS clang_rt.builtins-x86_64 PATH_SUFFIXES "lib/clang/*/lib/linux")
where * is an attempt to enable globbing (it's an unknown version number). It's looking for literal * in the paths it seems... replacing * with 12.0.0 works in my case, but that's not great. Let me see if LLVM actually exports a cmake target for this lib.
There was a problem hiding this comment.
Doesn't look like it.
|
I am trying to build with latest AOMP and am running into this issue. Can we please merge @haampie earlier patch ? |
|
ubuntu 20 requires that one be in the render group |
|
yes I am in both video and render: |
|
Closing all existing pull requests that do not target the develop branch. In case this pull request is still valid, please rebase your changes against develop branch and resubmit. |
6 participants
Ref this comment
#2217 (comment):