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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Reformat the code



================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:498
   /// far.
+
   class OffloadEntriesInfoManagerTy {
----------------
Remove unnecessary formatting changes. 


================
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);
----------------
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


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:2487
+llvm::GlobalVariable *
+createGlobalStruct(CodeGenModule &CGM, QualType Ty, bool IsConstant,
+                   ArrayRef<llvm::Constant *> Data, const Twine &Name,
----------------
Same here, make it protected or just create a copy, if it is small.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h:29-31
+  int ParallelLevel = 0;
+  int MaxParallelLevel = 0;
+  QualType TgtAttributeStructQTy;
----------------
Add comments for all new members


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h:97
+/// AMDGCN specific PrePostActionTy implementation
+class AMDGCNPrePostActionTy : public PrePostActionTy {
+    int &ParallelLevel;
----------------
1. Do you really need to make this class public?
2. `final`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86097