[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-15 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. In D58074#1399881 , @lildmh wrote: > Thanks for the catch! I also change the name from `OMPMappableExprListLocTy` > to `OMPVarListLocTy` to be more

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-15 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: include/clang/Sema/Sema.h:9347 ArrayRef MapTypeModifiersLoc, + CXXScopeSpec , DeclarationNameInfo , OpenMPMapClauseKind MapType, bool IsMapTypeImplicit, ABataev wrote: > Also would be good to pack

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-15 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: include/clang/AST/OpenMPClause.h:3666 + OMPMappableExprListClause( + OpenMPClauseKind K, OMPMappableExprListLocTy Locs, + OMPMappableExprListSizeTy Sizes, Pass those new structures by const reference, not by

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-15 Thread Lingda Li via Phabricator via cfe-commits
lildmh added inline comments. Comment at: include/clang/AST/OpenMPClause.h:3682-3685 + OpenMPClauseKind K, SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation EndLoc, OMPMappableExprListSizeTy Sizes, + NestedNameSpecifierLoc *MapperQualifierLocP =

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-15 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: include/clang/AST/OpenMPClause.h:3626 + struct OMPMappableExprListSizeTy { +unsigned NumVars; +unsigned NumUniqueDeclarations; Add default initializers for the fields Comment at:

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-14 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 186912. lildmh added a comment. Introduce a structure `OMPMappableExprListSizeTy` within `OMPMappableExprListClause` to aggregate all 4 sizes needed by such clause, and thus reduce the number of parameters when creating a map clause. This can also be used

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: include/clang/AST/OpenMPClause.h:4193 ArrayRef MapModifiersLoc, +NestedNameSpecifierLoc MapperQualifierLoc, +DeclarationNameInfo MapperIdInfo,

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-14 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked an inline comment as done. lildmh added a comment. Sure I'll add a codegen test with mapper. Thanks! Comment at: include/clang/AST/OpenMPClause.h:4193 ArrayRef MapModifiersLoc, +NestedNameSpecifierLoc

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D58074#1398187 , @lildmh wrote: > Hi Alexey, > > Again thanks for your review! The codegen completely ignores any mapper > related info for now, so it should not crash map clause codegen. It also > passed the regression test,

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-14 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked an inline comment as done. lildmh added a comment. Hi Alexey, Again thanks for your review! The codegen completely ignores any mapper related info for now, so it should not crash map clause codegen. It also passed the regression test, so map clause codegen should be fine.

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. Check, please, that adding this to the map clause does not crash the codegen. Would be good to ignore this construct in codegen for now, if it is used in the code. Comment at: include/clang/AST/OpenMPClause.h:4193 ArrayRef

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-13 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 186742. lildmh added a comment. 1. Move mapper related info to `OMPMappableExprListClause`, so that `to` and `from` clauses can utilize the same infrastructure as well (real support for them will be in another patch); 2. Combine the function parameters for

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-13 Thread Lingda Li via Phabricator via cfe-commits
lildmh added a reviewer: kkwli0. lildmh marked 4 inline comments as done. lildmh added inline comments. Comment at: lib/Parse/ParseOpenMP.cpp:2144 +parseMapType(*this, Data); } if (Data.MapType == OMPC_MAP_unknown) { kkwli0 wrote: > Although it

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Kelvin Li via Phabricator via cfe-commits
kkwli0 added inline comments. Comment at: include/clang/AST/OpenMPClause.h:4077 + /// C++ nested name specifier for the assoicated user-defined mapper. + NestedNameSpecifierLoc MapperQualifierLoc; assoicated -> associated Comment at:

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: lib/Sema/SemaLookup.cpp:1074 + if (NameKind == LookupOMPMapperName) { +// Skip out-of-scope declarations. lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > > > lildmh wrote: > > > > >

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked an inline comment as done. lildmh added inline comments. Comment at: lib/Sema/SemaLookup.cpp:1074 + if (NameKind == LookupOMPMapperName) { +// Skip out-of-scope declarations. ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > lildmh

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: lib/Sema/SemaLookup.cpp:1074 + if (NameKind == LookupOMPMapperName) { +// Skip out-of-scope declarations. lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > > > lildmh wrote: > > > > >

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked an inline comment as done. lildmh added inline comments. Comment at: lib/Sema/SemaLookup.cpp:1074 + if (NameKind == LookupOMPMapperName) { +// Skip out-of-scope declarations. ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > lildmh

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: lib/Sema/SemaLookup.cpp:1074 + if (NameKind == LookupOMPMapperName) { +// Skip out-of-scope declarations. lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > > > Why do we need special

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked 2 inline comments as done. lildmh added inline comments. Comment at: include/clang/AST/OpenMPClause.h:4236 + static OMPMapClause * + Create(const ASTContext , SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation EndLoc, ArrayRef Vars,

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D58074#1394993 , @lildmh wrote: > Hi Alexey, > > Thanks very much for your quick review! > > For the codegen, currently I'm thinking to do it within declare target > codegen functions. So there is not a very clear interface to

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 186492. lildmh marked 3 inline comments as done. lildmh added a comment. Fix part of Alexey's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58074/new/ https://reviews.llvm.org/D58074 Files: include/clang/AST/OpenMPClause.h

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked 11 inline comments as done. lildmh added a comment. Hi Alexey, Thanks very much for your quick review! For the codegen, currently I'm thinking to do it within declare target codegen functions. So there is not a very clear interface to do mapper codegen (also, there is not a

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. Herald added a subscriber: jdoerfert. Also, need some kind of the stubbed codegen for the mapper Comment at: include/clang/AST/OpenMPClause.h:4236 + static OMPMapClause * + Create(const ASTContext , SourceLocation StartLoc, SourceLocation LParenLoc,

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-11 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 186338. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58074/new/ https://reviews.llvm.org/D58074 Files: include/clang/AST/OpenMPClause.h include/clang/Basic/DiagnosticParseKinds.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

2019-02-11 Thread Lingda Li via Phabricator via cfe-commits
lildmh created this revision. lildmh added reviewers: ABataev, hfinkel, Meinersbur. lildmh added a project: OpenMP. Herald added subscribers: cfe-commits, guansong. Herald added a project: clang. This patch implements the parsing and sema support for OpenMP map clauses with potential