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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits