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

[Lingda Li via Phabricator via cfe-commits]

lildmh added inline comments.


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


================
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;
----------------
ABataev wrote:
> 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)`?
In libomptarget, the same way is used to define `OMP_TGT_MAPTYPE_MEMBER_OF`: `OMP_TGT_MAPTYPE_MEMBER_OF       = 0xffff000000000000`. So I think they should stay the same. Btw, the number 48 is directly used in libomptarget now, which may need to change in the future.

In your code, it assumes bits higher than 48 are all `OMP_MAP_MEMBER_OF`, which may not be true in the future. My code here is more universal, although it does not look great. What do you think?


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

https://reviews.llvm.org/D59474