[PATCH] D89994: [libomptarget][nvptx] Undef, internal shared variables

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:2858
     TransferMedium = new llvm::GlobalVariable(
-        M, Ty, /*isConstant=*/false, llvm::GlobalVariable::CommonLinkage,
-        llvm::Constant::getNullValue(Ty), TransferMediumName,
+        M, Ty, /*isConstant=*/false, llvm::GlobalVariable::InternalLinkage,
+        llvm::UndefValue::get(Ty), TransferMediumName,
----------------
"Internalization" is not the best option, it increases mem pressure. Common linkage is a better choice, allows to "squash" the same objects, defined in different units. Make it arch dependable, maybe?
For NVPTX zero initialization is not a problem, it is resolved when PTX is generated.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:4794
           CGM.getModule(), LLVMStaticTy,
-          /*isConstant=*/false, llvm::GlobalValue::CommonLinkage,
-          llvm::Constant::getNullValue(LLVMStaticTy),
+          /*isConstant=*/false, llvm::GlobalValue::InternalLinkage,
+          llvm::UndefValue::get(LLVMStaticTy),
----------------
JonChesterfield wrote:
> Perhaps weak_any + undef?
> 
> Could use internal for symbols that may vary in size and weak_any for those that don't.
Yeah, it is a good idea, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89994