[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/CGDecl.cpp:2533
+                                         CodeGenFunction *CGF) {
+  if (!LangOpts.OpenMP || LangOpts.OpenMPSimd ||
+      (!LangOpts.EmitAllDecls && !D->isUsed()))
----------------
Why do we need to emit it for simd only mode?


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1695
+  if (FunctionUDMMap.count(CGF.CurFn) > 0) {
+    for(auto *D : FunctionUDMMap[CGF.CurFn])
+      UDMMap.erase(D);
----------------
You're looking for `CGF.CurFn` twice here, used `find` member function instead and work with iterator.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7116-7124
+  /// Get the offset of the OMP_MAP_MEMBER_OF field.
+  static unsigned getFlagMemberOffset() {
+    unsigned Offset = 0;
+    for (uint64_t Remain = OMP_MAP_MEMBER_OF; !(Remain & 1);
+         Remain = Remain >> 1)
+      Offset++;
+    return Offset;
----------------
Maybe it is better to define a constant `constexpr uint64_t OMP_MEMBER_OF_RANK = 48` and then deduce `OMP_MAP_MEMBER_OF` as `~((1<<OMP_MEMBER_OF_RANK) - 1)`?


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8116
+                                MapFlagsArrayTy &Types) const {
+    // FIXME: MSVC 2013 seems to require this-> to find member CurDir.
+    assert(this->CurDir.is<const OMPDeclareMapperDecl *>() &&
----------------
AFAIK, LLVM has dropped support for msvc 2013, do we still need this?


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:352
+  /// Map of functions and their local user-defined mappers.
+  typedef llvm::DenseMap<llvm::Function *,
+                         SmallVector<const OMPDeclareMapperDecl *, 4>>
----------------
Use `using` instead of `typedef`.


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

https://reviews.llvm.org/D59474