ABataev added inline comments.
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068
+ unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok));
+ if (Type < OMPC_MAP_MODIFIER_unknown)
+ return OMPC_MAP_MODIFIER_unknown;
+ return static_cast<OpenMPMapModifierKind>(Type);
----------------
jdenny wrote:
> ABataev wrote:
> > Why do we need this?
> When called with `OMPC_map`, `getOpenMPSimpleClauseType` can return either an
> `OpenMPMapModifierKind` or `OpenMPMapClauseKind` depending on `Tok`. Thus,
> without this change, both `isMapModifier` and `isMapType` can return either
> of those despite the difference in their names and documentation.
>
> I found that, when `Parser::parseMapTypeModifiers` ignores `present` as if
> it's not a modifier because OpenMP < 5.1, then `isMapType` later returns
> `OMPC_MAP_MODIFIER_present` and causes the following assert to fail in
> `Sema::ActOnOpenMPVarListClause`:
>
> ```
> assert(0 <= ExtraModifier && ExtraModifier <= OMPC_MAP_unknown &&
> "Unexpected map modifier.");
> ```
>
> To me, the most obvious solution is to fix `isMapType` and `isMapModifier`.
> Fortunately, looking in `OpenMPKinds.h`, the enumerator values in
> `OpenMPMapModifierKind` and `OpenMPMapClauseKind` are disjoint so we can tell
> which we have.
Can we have something like:
```
if (LangOpts.OpenMP <= 50 && Type == OMPC_MAP_MODIFIER_present)
return OMPC_MAP_MODIFIER_unknown;
```
or extend `getOpenMPSimpleClauseType` function with the version parameter and
check there is modifier is allowed and return `unknown` if it is not compatible
with provided version?
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3103
/// map([ [map-type-modifier[,] [map-type-modifier[,] ...] map-type : ] list)
/// where, map-type-modifier ::= always | close | mapper(mapper-identifier)
bool Parser::parseMapTypeModifiers(OpenMPVarListDataTy &Data) {
----------------
Update a comment here
================
Comment at: clang/test/OpenMP/target_data_codegen.cpp:258-260
+// CK1A: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1021]]]
+
+// CK1A: [[MTYPE01:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1425]]]
----------------
Add a comment with interpretaion of the map flags, like `OMP_TO = 0x1 |
OMP_FROM=0x2 | OMP_PRESENT = 0x1000 = 0x1003`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83061/new/
https://reviews.llvm.org/D83061
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
