ABataev added inline comments.
================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:2343-2344 + // pointer + if (Arg.back() < OMPC_DEFAULTMAP_MODIFIER_unknown) + Arg.back() = OMPC_DEFAULTMAP_MODIFIER_unknown; KLoc.push_back(Tok.getLocation()); ---------------- Is this possible at all? I think it must an assertion rather this kind of trick. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:651 + if (DMVC == DMVC_scalar || DMVC == DMVC_pointer) { + return (getDefaultDMIBAtLevel(Level, DMVC) == + DMIB_alloc) || ---------------- Cache the result of `getDefaultDMIBAtLevel(Level, DMVC)` in a var, do not call it several times. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:663 + bool isDefaultmapDefaultBehavior(DefaultMapVariableCategory DMVC) const { + return (getDefaultDMIB(DMVC) == DMIB_unspecified) || + (getDefaultDMIB(DMVC) == DMIB_firstprivate) || ---------------- Same, cache the result of the call ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:668 + bool isDefaultmapDefaultBehaviorAtLevel(unsigned Level, + DefaultMapVariableCategory DMVC) const { + return (getDefaultDMIBAtLevel(Level, DMVC) == DMIB_unspecified) || ---------------- Seems to me, the code is not formatted. Use clang-format. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:669 + DefaultMapVariableCategory DMVC) const { + return (getDefaultDMIBAtLevel(Level, DMVC) == DMIB_unspecified) || + (getDefaultDMIBAtLevel(Level, DMVC) == DMIB_firstprivate) || ---------------- same here ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:2843 llvm::SmallVector<Expr *, 4> ImplicitFirstprivate; - llvm::SmallVector<Expr *, 4> ImplicitMap; + llvm::SmallVector<Expr *, 4> ImplicitMap[DMIB_none]; Sema::VarsWithInheritedDSAType VarsWithInheritedDSA; ---------------- I would prefer to use `OpenMPMapClauseKind` type as a key here to prevent all those conversions of the default (none, default, unknown) mappings to real mappings. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:2987 if (IsFirstprivate) ImplicitFirstprivate.emplace_back(E); + else { ---------------- Enclose this code into braces too, if the code in `else` branch is enclosed in braces the code in `then` branch also must be enclosed in braces. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:2990 + DefaultMapImplicitBehavior DMIB = Stack->getDefaultDMIB(DMVC); + if (DMIB >= DMIB_none) { + if (DMVC == DMVC_aggregate || Res) DMIB = DMIB_tofrom; ---------------- Better to use switches rather such kind of comparisons for enums. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:2991-2992 + if (DMIB >= DMIB_none) { + if (DMVC == DMVC_aggregate || Res) DMIB = DMIB_tofrom; + else llvm_unreachable("Unexpected defaultmap implicit behavior"); + } ---------------- Format the code. 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