[PATCH] D95558: [NFC][CUDA] Refactor registering device variable

[Yaxun Liu via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:157
   llvm::Function *makeModuleDtorFunction() override;
+  void
+  adjustShadowVarLinkage(const VarDecl *D,
----------------
tra wrote:
> clang-format it? `void` hanging all by itself looks odd.
Although it looks strange, it is a result of clang-format.


================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:924
+
+void CGNVCUDARuntime::adjustShadowVarLinkage(
+    const VarDecl *D, llvm::GlobalValue::LinkageTypes &Linkage) {
----------------
tra wrote:
> The name and the functionality do not match -- despite what the name says, the function will set internal linkage on any var with device-side attributes.
> 
> Rename to `internalizeDeviceSideVars` ?
done


================
Comment at: clang/lib/CodeGen/CGCUDARuntime.h:86
+  /// Check whether a variable is a device variable and register it if true.
+  virtual void mayRegisterDeviceVar(const VarDecl *VD,
+                                    llvm::GlobalVariable &Var) = 0;
----------------
tra wrote:
> `handleVarRegistration` to make it distinct from the implementation of specific registration kinds.
> 
> Maybe, move `registerDevice*` functions under `private:` as they will only be used from the top-level handler called by CGM.
done


================
Comment at: clang/lib/CodeGen/CGCUDARuntime.h:89
+  /// Register regular device variable (not surface or texture).
+  virtual void registerDeviceVarRegular(const VarDecl *VD,
+                                        llvm::GlobalVariable &Var, bool Extern,
----------------
tra wrote:
> Just `registerDeviceVar` would do.
done


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

https://reviews.llvm.org/D95558