[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:473
+  /// atomic_default_mem_order seq_cst clause.
+  OMP_REQ_ATOMIC_DEFAULT_SEQ_CST  = 0x008,
+  /// atomic_default_mem_order acq_rel clause.
----------------
YOu don't need al these flags, add only target-specific.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1254
+
+  HasRequiresUnifiedSharedMemory = false;
 }
----------------
No need to do this initialization here


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8978
+    if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
+      CGM.getOpenMPRuntime().HasRequiresUnifiedSharedMemory = true;
+      break;
----------------
Just `HasRequiresUnifiedSharedMemory = true;`


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9045
+  // don't need to do anything.
+  if (CGM.getLangOpts().OpenMPIsDevice || OffloadEntriesInfoManager.empty())
+    return nullptr;
----------------
Just add a check for OpenMPSimd mode here and do not emit anything in this case.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9061
+    //TODO: check for other requires clauses.
+    if (HasRequiresUnifiedSharedMemory) {
+      Flags |= OMP_REQ_UNIFIED_SHARED_MEMORY;
----------------
No need for braces


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9065
+    CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__tgt_register_requires),
+        llvm::ConstantInt::get(CGF.CGM.Int64Ty, Flags));
+    CGF.FinishFunction();
----------------
`CGF.CGM.Int64Ty`->`CGM.Int64Ty`


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:1438
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
----------------
Why do you need this vertual funtion? I think static local is going to be enough


================
Comment at: lib/CodeGen/CodeGenModule.cpp:415
+            OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+      AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+    }
----------------
You can remove the third `nullptr` argument


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568