ABataev added inline comments.
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2020-2034
if (!FirstClause) {
Diag(Tok, diag::err_omp_more_one_clause)
<< getOpenMPDirectiveName(DKind) << getOpenMPClauseName(CKind) << 0;
ErrorFound = true;
}
+ Clause = ParseOpenMPSingleExprWithArgClause(CKind, WrongDirective);
+ break;
----------------
I think better just to modify the condition in the original `if`:
```
if ((getLangOpts().OpenMP < 50 || CKind != OMPC_defaultmap) && !FirstClause)
```
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:118
+ DefaultmapInfo(OpenMPDefaultmapClauseModifier M,
+ OpenMPDefaultmapClauseKind Kind, SourceLocation Loc)
+ : ImplicitBehavior(M), SLoc(Loc) {}
----------------
`Kind` is unused here, do you really need it?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2823
+static OpenMPMapClauseKind
+getMapClauseKindFromModifier(OpenMPDefaultmapClauseModifier M, bool Cond) {
+ OpenMPMapClauseKind Kind = OMPC_MAP_unknown;
----------------
Better to rename `Cond` to something like `IsAggregateOrDeclareTarget`
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2847-2851
+ if (Cond)
+ Kind = OMPC_MAP_tofrom;
+ else
+ llvm_unreachable("Unexpected defaultmap implicit behavior");
+ break;
----------------
```
if (Cond) {
Kind = OMPC_MAP_tofrom;
break;
}
llvm_unreachable("Unexpected defaultmap implicit behavior");
```
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2953-2954
+ (!(isOpenMPTargetExecutionDirective(DKind) && Res &&
+ (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+ *Res == OMPDeclareTargetDeclAttr::MT_To)))) {
+ // Only check for data-mapping attribute and is_device_ptr here
----------------
Why comparing the value of `*Res` here if just comparing against all possible
values? This condition is always `true` if `Res` is true. I don't think we need
to map variables with `to` mapping.
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
