[Openmp-commits] [PATCH] D133539: [OpenMP] Replace OpenMP register requires constructor with a global array

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

Generally happy with this. Some minor comments. @sandoval, this works for you?



================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3020-3025
+  if (!RequiresFlags.empty()) {
+    for (int64_t Flag : RequiresFlags)
+      createRegisterRequires(CGM, Flag);
+  } else {
+    createRegisterRequires(CGM, OMP_REQ_NONE);
+  }
----------------



================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10515
+    default:
+      RequiresFlags.push_back(OMP_REQ_NONE);
+    }
----------------
Really? Not an error or unexpected state?


================
Comment at: openmp/libomptarget/src/rtl.cpp:360
+  if (Desc->Version >= 0)
     return;
 
----------------
Why do we allow to be called with an old struct, wouldn't we have converted it at this point?


================
Comment at: openmp/libomptarget/src/rtl.cpp:364
+               *RequiresE = Desc->RegisterRequiresEnd;
+       RequiresB != RequiresE; RequiresB++) {
+    int64_t Flags = *RequiresB;
----------------
We have llvm::make_range, or similar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133539