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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to