[PATCH] D155850: [Clang][CodeGen][RFC] Add codegen support for C++ Parallel Algorithm Offload

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 21 10:35:30 PDT 2023


yaxunl added a comment.

In D155850#4523051 <https://reviews.llvm.org/D155850#4523051>, @AlexVlx wrote:

> @yaxunl interesting point - are you worried about cases where due to missing inlining / const prop an indirect call site that can be replaced with a direct one would remain indirect? I think the problem in that case would actually be different, in that possibly reachable functions would not be identified as such and would be erroneously removed. I'm not sure there's any case where we'd fail to remove a meant to be unreachable function. We can definitely go with the `__clang_unsupported` approach, but I think I'd prefer these to be compile time errors rather than remarks + runtime `printf`, not in the least because `printf` adds some overhead. A way to ensure we don't "miss a spot" might be to check after removal for any remaining unsupported builtins, instead of doing it during reachability computation (this is coupled with the special naming from the prior post).

For programs having multiple TUs we cannot decide whether an unsupported function is used by a kernel during the compilation of a single TU. We can only decide that when we have the IR for the whole program. Currently, the HIP toolchain uses LTO of lld for multiple TUs, I am not sure whether we can emit clang diagnostics from lld. If not, then we need to use remarks. If we are confident to remove most unreachable unsupported functions at -O0, we may not need to use printf at run time. Remarks at LTO should be sufficient.

  if (foundGPU())
    func_use_amdgpu_builtin();
  else
    func_use_x64_builtin();



In D155850#4523051 <https://reviews.llvm.org/D155850#4523051>, @AlexVlx wrote:

> @yaxunl interesting point - are you worried about cases where due to missing inlining / const prop an indirect call site that can be replaced with a direct one would remain indirect? I think the problem in that case would actually be different, in that possibly reachable functions would not be identified as such and would be erroneously removed. I'm not sure there's any case where we'd fail to remove a meant to be unreachable function. We can definitely go with the `__clang_unsupported` approach, but I think I'd prefer these to be compile time errors rather than remarks + runtime `printf`, not in the least because `printf` adds some overhead. A way to ensure we don't "miss a spot" might be to check after removal for any remaining unsupported builtins, instead of doing it during reachability computation (this is coupled with the special naming from the prior post).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155850



More information about the cfe-commits mailing list