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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to