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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 2 inline comments as done.
cchen 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);
----------------
ABataev wrote:
> I believe this problem exists in the original code? If so, better to split this patch and commit this fix in the separate patch.
Yes, this problem is from the original code, but `bool isDefaultmapModifier = (M != OMPC_DEFAULTMAP_MODIFIER_unknown);` at line 16437 relies on this change. Otherwise, I'll have to write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || (M == OMPC_DEFAULTMAP_MODIFIER_from) ||...`.
So do you think that I should just write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || (M == OMPC_DEFAULTMAP_MODIFIER_from) ||...` or `M > OMPC_DEFAULTMAP_MODIFIER_unknown` for this patch and fix them in another 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;
----------------
ABataev wrote:
> 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?
Got it, I'll use OpenMPDefaultmapClauseModifier and OpenMPDefaultmapClauseKind instead. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204