[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

dmajor added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: erik.pilkington. While debugging something else, I noticed that with this patch, `Decl` now has 33 bits worth of bitfields, so it has gained an extra word. Is that ok? Just want to make sure it wasn't unintent

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh added a comment. Hi David, An extra bit is needed to be added to `IdentifierNamespace`, and I could not find another field to shrink its width. As a result, it ends up larger than a word now. If you have a better solution, I'll be happy to know. Thanks, Lingda Repository: rC Clang

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh created this revision. Herald added subscribers: cfe-commits, jfb, arphaman, guansong, jholewinski. Repository: rC Clang https://reviews.llvm.org/D56326 Files: include/clang/AST/DeclBase.h include/clang/AST/DeclCXX.h include/clang/AST/DeclOpenMP.h include/clang/AST/RecursiveASTV

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); No, bad idea. Use tail allocation for the clauses. Check the implementation

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh marked 11 inline comments as done. lildmh added a comment. Hi Alexey, Thanks a lot for the review! For 3 places I have doubt. Please see the comments inline. I tried to fix the rest of your comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clau

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh updated this revision to Diff 181289. lildmh marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326 Files: include/clang/AST/DeclBase.h include/clang/AST/DeclCXX.h include/clang/AST/DeclOpenMP.h include/cl

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); lildmh wrote: > ABataev wrote: > > No, bad idea. Use tail allocation for the

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh marked an inline comment as done. lildmh added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); ABataev wrote: > lildmh wrote: > > A

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh marked an inline comment as done. lildmh added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); ABataev wrote: > lildmh wrote: > > A

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > lildmh wrote: > > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > lildmh wrote: > > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > ABataev wrote: > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh added inline comments. Comment at: lib/AST/DeclOpenMP.cpp:164 + if (NumClauses) { +Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses); +setClauses(CL); ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > lildmh wrote: > > > >

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh updated this revision to Diff 183570. lildmh added a comment. Change the type of mapper clause storage from OMPClause ** to MutableArrayRef, and rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326 Files: include/clang/AST/DeclBase.h

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added inline comments. Comment at: include/clang/AST/DeclBase.h:181 +/// This declaration is an OpenMP user defined mapper. +IDNS_OMPMapper = 0x2000 }; Add comma after `0x2000` to exclude this line from the next pactches ===

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh marked 18 inline comments as done. lildmh added a comment. Hi Alexey, Thanks a lot for the review! Tried to address all of your comments in the new diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh updated this revision to Diff 183615. lildmh added a comment. Address review comments and rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326 Files: include/clang/AST/DeclBase.h include/clang/AST/DeclCXX.h include/clang/AST/DeclO

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev added a comment. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh updated this revision to Diff 184740. lildmh added a comment. Herald added a subscriber: openmp-commits. Rebase. Hi Alexey, Thanks a lot for the review! It seems no one else will review it. If you think it's ready to go, could you please accept this patch? I also need help to commit thi

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. Accepted, just was not marked as accepted for an unknown reason. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh added a comment. Thanks a lot Alexey! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56326/new/ https://reviews.llvm.org/D56326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

Meinersbur added a comment. @lildmh Thanks for the patch. I can commit this patch for you. The tests run fine on my machine. However, for committing on other's behalf, we were asked to make the contributor aware that the contribution will be published under a new license

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

lildmh added a comment. In D56326#1381229 , @Meinersbur wrote: > @lildmh Thanks for the patch. I can commit this patch for you. The tests run > fine on my machine. However, for committing on other's behalf, we were asked > to make the contributor aware t

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

This revision was automatically updated to reflect the committed changes. Closed by commit rC352906: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive. (authored by Meinersbur, committed by ). Herald added a project: clang. Repository: rC Clang CHANGES SINCE LAST ACTION ht