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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to