[PATCH] D155856: [HIP][LLVM][Opt][RFC] Add LLVM support for C++ Parallel Algorithm Offload

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl added a comment.

Are these passes called when compiling C++ to bitcode? If so, then device functions not called by kernels in the same module will be removed, right? Then we cannot support calling device functions in a different TU. Should these passes be moved to the llvm codegen pipelines so that they will only be called post-linking?



================
Comment at: llvm/lib/Transforms/HipStdPar/HipStdPar.cpp:62
+template<typename T>
+static inline void eraseFromModule(T &toErase) {
+  toErase.replaceAllUsesWith(PoisonValue::get(toErase.getType()));
----------------
variable names should be capitalized https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly


================
Comment at: llvm/lib/Transforms/HipStdPar/HipStdPar.cpp:73
+
+  if (!G.isConstantUsed())
+    return true;
----------------
Is this condition correct?

AMDGPU backend supports non-constant TLS?


================
Comment at: llvm/lib/Transforms/HipStdPar/HipStdPar.cpp:135
+
+    G.setLinkage(GlobalVariable::ExternalWeakLinkage);
+    G.setExternallyInitialized(true);
----------------
Any reason to change the linkage to weak?


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

https://reviews.llvm.org/D155856