[PATCH] D86097: [OpenMP][AMDGCN] Generate global variables and attributes for AMDGCN

Wed Aug 26 12:27:57 PDT 2020

saiislam marked an inline comment as done.
saiislam added inline comments.

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:2479-2483
+/// Declaration of functions visible in clang::CodeGen namespace, to
+/// be used by target specific specializations of CGOpenMPRuntimeGPU.
+
+FieldDecl *addFieldToRecordDecl(ASTContext &C, DeclContext *DC,
+                                       QualType FieldTy);
----------------
ABataev wrote:
> Better to make it a protected member function if you really require it. Plus, this function is very small and, I think, you simply create your own copy in CGOpenMPRuntimeAMDGCN
Not making it protected because it is used by various static functions. And don't want to create an object pointer of subclass of CGOpenMPRuntime in CGOpenMPRuntime.

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:2487
+llvm::GlobalVariable *
+createGlobalStruct(CodeGenModule &CGM, QualType Ty, bool IsConstant,
+                   ArrayRef<llvm::Constant *> Data, const Twine &Name,
----------------
ABataev wrote:
> Same here, make it protected or just create a copy, if it is small.
It calls static functions which in turn call other static functions, so it won't make sense to create a copy of whole function chain in amdgcn.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86097