[PATCH] D99683: [HIP] Support ThinLTO

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D99683#2669080 <https://reviews.llvm.org/D99683#2669080>, @tejohnson wrote:

> In D99683#2669047 <https://reviews.llvm.org/D99683#2669047>, @yaxunl wrote:
>
>> In D99683#2664674 <https://reviews.llvm.org/D99683#2664674>, @tejohnson wrote:
>>
>>> I haven't looked extensively yet, but why import noinline functions? Also, please add a patch description.
>>
>> AMDGPU backend does not support linking of object files containing external symbols, i.e. one object file calling a function defined in another object file. Therefore the LLVM module passed to AMDGPU backend needs to contain definitions of all callees, even if a callee has noinline attribute. To support backends like this, the function importer needs to be able to import functions with noinline attribute. Therefore we add an LLVM option for allowing that, which is off by default. We have comments at line 70 of HIP.cpp about this.
>
> How does a non-LTO build work, or is (full) LTO currently required? Because with ThinLTO we only import functions that are externally defined but referenced in the current module. Also, when ThinLTO imports functions it makes them available_externally, which means they are dropped and made external symbols again after inlining. So anything imported but not inlined will go back to being an external symbol.

AMDGPU backend by default uses full LTO for linking. It does not support non-LTO linking. Currently, it inlines all functions except kernels. However we want to be able to be able not to inline all functions. Is it OK to add an LLVM option to mark imported functions as linkonce_odr so that AMDGPU backend can keep the definitions of the imported functions?


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

https://reviews.llvm.org/D99683