[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D115283#3183034 <https://reviews.llvm.org/D115283#3183034>, @JonChesterfield wrote:

> In D115283#3182879 <https://reviews.llvm.org/D115283#3182879>, @yaxunl wrote:
>
>> In D115283#3181128 <https://reviews.llvm.org/D115283#3181128>, @JonChesterfield wrote:
>>
>>> Not exactly that. The weak symbol isn't the function name, as that gets renamed or inlined.
>>
>> We discussed this before. As code object ABI use runtime metadata to represent hostcall_buffer, we need to check whether hostcall is needed by IR.
>>
>> This approach will require checking asm instructions inside a function to determine whether this function requires hostcall. It is hacky for IR representation.
>
> There are two approaches here:
> 1/ Tag the function using inline asm and totally ignore it in the compiler. HSA/etc tests per-code-object if the symbol is present
> 2/ Tag the function (in source or in compiler), propagate information to llc, embed it in msgpack data, HSA/etc tests per-function if the field is present
>
> 2/ is somewhat useful if we elide the 8 byte slot of kernarg memory for functions that don't use it, otherwise it just increases work done by the runtime. Instead of checking for presence of one symbol (a hashtable lookup), it's a linear scan through msgpack data. We don't currently elide those 8 bytes, so right now this is making the compiler more complicated in exchange for making the runtime slower.
>
> 1/ has the benefit of being dead simple and totally compiler agnostic, and the cost of passing the 8 byte hostcall thing to every function in a code object that asked for it.

Option 1 needs code object ABI change that does not work with old ROCm runtime. We need to maintain certain stability and backward compatibility with old ROCm as we have customers who use trunk clang/llvm with older ROCm runtime.

We could discuss option 1 for the next version of code object format. However, before that happens, we still need to fix the bug within the current ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283