[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345
+    unsigned Modifier = getOpenMPSimpleClauseType(
+        Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+    // Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+    // pointer
+    if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+      Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+    Arg.push_back(Modifier);
----------------
I believe this problem exists in the original code? If so, better to split this patch and commit this fix in the separate patch.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16393-16416
+static DefaultMapImplicitBehavior
+getImplicitBehaviorFromModifier(OpenMPDefaultmapClauseModifier M) {
+  switch (M) {
+  case OMPC_DEFAULTMAP_MODIFIER_alloc:
+    return DMIB_alloc;
+  case OMPC_DEFAULTMAP_MODIFIER_to:
+    return DMIB_to;
----------------
cchen wrote:
> ABataev wrote:
> > ABataev wrote:
> > > Do we need types `DefaultMapImplicitBehavior` and `DefaultMapVariableCategory` if the map 1 to 1 to `OpenMPDefaultmapClauseModifier` and `OpenMPDefaultmapClauseKind`?
> > What about this?
> The value of OpenMPDefaultmapClauseModifier does not start from one (due to the design in parsing I guess) so I'm not able to do so.
No, I meant that the enumerics are almost the same. Can we reuse the original one rather than adding 2 new enums with almost the same members and the same meanings?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3013-3020
+          IsFirstprivate = IsFirstprivate ||
+                           (((DMVC == DMVC_pointer &&
+                              Stack->mustBeFirstprivate(DMVC_pointer)) ||
+                             (DMVC == DMVC_scalar &&
+                              Stack->mustBeFirstprivate(DMVC_scalar)) ||
+                             (DMVC == DMVC_aggregate &&
+                              Stack->mustBeFirstprivate(DMVC_aggregate))) &&
----------------
`Stack->mustBeFirstprivate(DMVC)`?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4448
+          DSAChecker.getImplicitMap(static_cast<DefaultMapVariableCategory>(I));
+      for (const auto &M : ImplicitMap)
+        ImplicitMaps[I].emplace_back(M);
----------------
`ImplicitMaps[I].append(ImplicitMap.begin(), ImplicitMap.end());`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204