[PATCH] D96195: [HIP] Fix managed variable linkage

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:1017
+
+void CGNVCUDARuntime::transformManagedVars() {
+  for (auto &&Info : DeviceVars) {
----------------
tra wrote:
> A comment about how exactly we're transforming the vars would be helpful.
done


================
Comment at: clang/lib/CodeGen/CGCUDARuntime.h:107
+  /// Transform managed variables in device compilation.
+  virtual void transformManagedVars() = 0;
 };
----------------
tra wrote:
> Can we fold it into `makeModuleCtorFunction` ?  It does not look like something we would need to customize independently of other glue generating machinery.
> 
makeModuleCtorFunction is creating module ctor function, but transformManagedVars has nothing to do with module ctor function.

Also, makeModuleCtorFunction is called in host compilation only.

Do we want to rename the API here finalizeModule ? And let it call makeModuleCtorFunction and transformManagedVars as static functions in CGCUDANV.cpp ?


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:463-466
+    if (Context.getLangOpts().CUDAIsDevice)
+      CUDARuntime->transformManagedVars();
+    else if (llvm::Function *CudaCtorFunction =
+                 CUDARuntime->makeModuleCtorFunction())
----------------
tra wrote:
> This does not look right. 
> Was `transformManagedVars()` supposed to *replace* `AddGlobalCtor(CudaCtorFunction)`?
> 
> I would expect it to be in addition to the ctor we create now.
> 
> ```
>     if (Context.getLangOpts().CUDAIsDevice)
>       CUDARuntime->transformManagedVars();
>     if (llvm::Function *CudaCtorFunction =
>                  CUDARuntime->makeModuleCtorFunction())
>          AddGlobalCtor(CudaCtorFunction);
> ```
originally, makeModuleCtorFunction is called for host compilation only.


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

https://reviews.llvm.org/D96195