[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

Wed Jul 20 10:10:24 PDT 2022

yaxunl added a comment.

In D130096#3663411 <https://reviews.llvm.org/D130096#3663411>, @arsenm wrote:

> In D130096#3663398 <https://reviews.llvm.org/D130096#3663398>, @jhuber6 wrote:
>
>> In D130096#3663295 <https://reviews.llvm.org/D130096#3663295>, @yaxunl wrote:
>>
>>> There is no constant propagation for globals with weak linage, right? Otherwise, it won't work. My concern is that there may be optimization passes which do not respect the weak linkage and uses the incorrect default value for OpenCL or HIP. Therefore I am not very confident to enable this for OpenCL or HIP unless these variables have the correct value based on the compilation options.
>>
>> Instead of `weak_odr` we could probably use add this to compiler used instead if that's an issue.
>
> the libraries get internalized as-is. Why does this need to be weak_odr?

The current patch does not consider HIP/OpenCL compile options, therefore the value of these variables are not correct for OpenCL/HIP. They need to be overridden by the variables with the same name in device libraries by clang through -mlink-builtin-bitcode.

If the patch check HIP/OpenCL compilation options to set the correct value for these variables, then it does not need weak linkage.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130096/new/

https://reviews.llvm.org/D130096