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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3870
 llvm::Function *
+CGOpenMPRuntime::createRequiresDirectiveRegistration() {
+  // If we don't have entries or if we are emitting code for the device, we
----------------
ABataev wrote:
> Why do you need a new member function? Can you make a static local function?
Same as for register lib.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3887
+    CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {});
+    int64_t Flags = OMP_REQ_NONE;
+    //TODO: check for other requires clauses.
----------------
ABataev wrote:
> Use `OpenMPOffloadingRequiresDirFlags` instead of `int64_t`
Leads to error if I do that. This enum behaves like OpenMPLocationFlags.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7983
                                         MapFlagsArrayTy &Types) const {
+    // If using unified memory, no need to do the mappings.
+    if (CGF.CGM.HasRequiresUnifiedSharedMemory)
----------------
ABataev wrote:
> Seems to me, this must be in another patch, has nothing to do with this patch
I will split it.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9069
 
+llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() {
+  // Create and register the function that handles the requires directives.
----------------
ABataev wrote:
> Why do you need the second function?
Second function eliminated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568