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

[Jeff Sandoval via Phabricator via cfe-commits]

sandoval added a comment.

I don't see a clear explain the motivation for this change - can you confirm my understanding or provide clarification?  It looks like the issue is that D110337 <https://reviews.llvm.org/D110337> caused a regression for cases when user code directly calls a device library function that requires hostcall services, right?  If so, I think this issue highlights a weakness in the module flag approach implemented in D110337 <https://reviews.llvm.org/D110337> - i.e., now the compiler needs to know every library function that may require hostcall services.

We have this same issue with our proprietary compiler, where we have our own device runtime library that makes use of printf.  The prior approach of detecting the `ockl_hostcall_internal` function definition handles this case just fine (with the caveat of the potential LTO/inlining issues mentioned in D110337 <https://reviews.llvm.org/D110337>).  But with the new approach to use the `amdgpu_hostcall` module flag, we need to modify our compiler to emit that flag for all of our own library calls, too.

Another concern with using a module flag is that is isn't as easily eliminated once it has been inserted, even if the call that triggered insertion is ultimately eliminated through optimization.  E.g., a printf call might be eliminated if it is under a condition that can be statically evaluated to false...but, the `amdgpu_hostcall` module flag may already have been inserted.

Is there an approach that can avoid the LTO/inlining issues, like implementing the hostcall implementation with an intrinsic to access the hostcall buffer pointer? - then the AMDGPU backend could easily detect use of that intrinsic to trigger setup of the implicit kernel arg, and inlining could not eliminate that intrinsic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283