[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 closed this revision.
doru1004 added a comment.

Commit 49d47c4d2f280d15d1de94c53b72b6ab3c127b35 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 490659.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/declare_mapper_ast_print.c
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_map_messages.cpp

Index: clang/test/OpenMP/target_map_messages.cpp
===
--- clang/test/OpenMP/target_map_messages.cpp
+++ clang/test/OpenMP/target_map_messages.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -verify=expected,lt50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=45 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=50 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51,omp,ge51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=51 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,ge50,ge51,ge52,omp,ge52-omp -fopenmp -fno-openmp-extensions -fopenmp-version=52 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -DCCODE -verify -fopenmp -fno-openmp-extensions -ferror-limit 300 -x c %s -Wno-openmp -Wuninitialized
 
 // -fopenmp-simd, -fno-openmp-extensions
@@ -158,23 +159,28 @@
 // expected-error@+1 {{use of undeclared identifier 'present'}}
 #pragma omp target map(present)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[1:2],f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f[1:2])
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[:],f)
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
@@ -191,11 +197,15 @@
 // lt51-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(present, present, tofrom: a)
 {}
+// ge52-omp-error@+5 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ompx-error@+3 {{same map type modifier has been specified more than once}}
 // ge51-omp-error@+2 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 2 

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/declare_mapper_messages.c:33-36
+#pragma omp declare mapper(id2: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[it])
+int var; // expected-note {{'var' declared here}}
+// expected-error@+1 {{only variable 'vvec' is allowed in map clauses of this 
'omp declare mapper' directive}}
+#pragma omp declare mapper(id3: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[var])

doru1004 wrote:
> ABataev wrote:
> > doru1004 wrote:
> > > Here we have both the positive and the negative declare mapper cases. 
> > > Please let me know if you meant something different.
> > Would be good to have ast print case and codegen
> I can add ast print! Code gen for this map modifier is going to be done in a 
> separate patch.
Ok, a st print should be enough


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/declare_mapper_messages.c:33-36
+#pragma omp declare mapper(id2: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[it])
+int var; // expected-note {{'var' declared here}}
+// expected-error@+1 {{only variable 'vvec' is allowed in map clauses of this 
'omp declare mapper' directive}}
+#pragma omp declare mapper(id3: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[var])

ABataev wrote:
> doru1004 wrote:
> > Here we have both the positive and the negative declare mapper cases. 
> > Please let me know if you meant something different.
> Would be good to have ast print case and codegen
I can add ast print! Code gen for this map modifier is going to be done in a 
separate patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/declare_mapper_messages.c:33-36
+#pragma omp declare mapper(id2: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[it])
+int var; // expected-note {{'var' declared here}}
+// expected-error@+1 {{only variable 'vvec' is allowed in map clauses of this 
'omp declare mapper' directive}}
+#pragma omp declare mapper(id3: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[var])

doru1004 wrote:
> Here we have both the positive and the negative declare mapper cases. Please 
> let me know if you meant something different.
Would be good to have ast print case and codegen


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:5423-5426
+
+/// Act on the iterator variable declaration.
+ActOnOpenMPIteratorVarDecl(VD);
+

ABataev wrote:
> Can we register this variable only in declare mapper context, i.e. add a 
> check that we add it only for declare mapper?
Happy to do that!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/declare_mapper_messages.c:33-36
+#pragma omp declare mapper(id2: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[it])
+int var; // expected-note {{'var' declared here}}
+// expected-error@+1 {{only variable 'vvec' is allowed in map clauses of this 
'omp declare mapper' directive}}
+#pragma omp declare mapper(id3: struct vec vvec) 
map(iterator(it=0:vvec.len:2), tofrom:vvec.data[var])

Here we have both the positive and the negative declare mapper cases. Please 
let me know if you meant something different.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Can you add positive test for declare mapper?




Comment at: clang/lib/Sema/SemaExpr.cpp:5423-5426
+
+/// Act on the iterator variable declaration.
+ActOnOpenMPIteratorVarDecl(VD);
+

Can we register this variable only in declare mapper context, i.e. add a check 
that we add it only for declare mapper?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:22399-22412
 bool Sema::isOpenMPDeclareMapperVarDeclAllowed(const VarDecl *VD) const {
   assert(LangOpts.OpenMP && "Expected OpenMP mode.");
   const Expr *Ref = DSAStack->getDeclareMapperVarRef();
   if (const auto *DRE = cast_or_null(Ref)) {
 if (VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl())
   return true;
 if (VD->isUsableInConstantExpressions(Context))

This input to this function is the VD variable I've been talking about. If you 
print it all out it's just a simple VarDecl:

if you do `VD->dump()`;
```
VarDecl 0x55b57f81dec8  col:52 implicit 
used it 'int'
```

if you do `VD->getType()->dump()`:
```
BuiltinType 0x55b57f6d2560 'int'
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

doru1004 wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > doru1004 wrote:
> > > > doru1004 wrote:
> > > > > ABataev wrote:
> > > > > > doru1004 wrote:
> > > > > > > ABataev wrote:
> > > > > > > > doru1004 wrote:
> > > > > > > > > ABataev wrote:
> > > > > > > > > > Do you really need to store the variable in the stack, is 
> > > > > > > > > > not it enough just to check that the type of this variable 
> > > > > > > > > > is BuiltinType::OMPIterator?
> > > > > > > > > I'm happy to replace this if you think it will work. Is there 
> > > > > > > > > an example somewhere in the code where I can get from the 
> > > > > > > > > VarDecl to the build in type you mention?
> > > > > > > > You have already a check 
> > > > > > > > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> > > > > > > >  you can you something similar for the variable
> > > > > > > This didn't work and I had to revert to using the stack!
> > > > > > Why?
> > > > > I checked the output of the check and it was false when it should 
> > > > > have been true! If you check the latest test that I added it 
> > > > > showcases the source code and in the case of OpenMP 5.2 you can see 
> > > > > that the message "only variable 'vvec' is allowed in map clauses of 
> > > > > this 'omp declare mapper' directive" doesn't appear when a legal 
> > > > > iteration variable is used.
> > > > > If I used the check you suggested then the error message appears.
> > > > > In the example you pasted the check is performed on a `Expr *`. In 
> > > > > the case here, we only have VD which is a VarDecl.
> > > > I am not sure how I can force it to have that type when it just 
> > > > doesn't. Do you have any suggestions?
> > > Did not get it. It still shall be of type builtintype::OMPIterator.
> > The VD that we are checking for this builtin is coming from somewhere else 
> > in the code, it is passed into the `Sema::DiagnoseUseOfDecl(` function. 
> > It's not a VarDecl that is under the control of anything added in this 
> > patch.
> This implementation is in line with the current checks for the declaration of 
> the mapper variable. You store the declaration onto the stack so that you can 
> compare it with the incoming VarDecl passed to the diagnose function.
Some debug printouts regarding VD:

> VD->dump();
```
VarDecl 0x55b57f81dec8  col:52 implicit 
used it 'int'
```

This is the type of the variable if you do `VD->getType()->dump()`:
```
BuiltinType 0x55b57f6d2560 'int'
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

doru1004 wrote:
> ABataev wrote:
> > doru1004 wrote:
> > > doru1004 wrote:
> > > > ABataev wrote:
> > > > > doru1004 wrote:
> > > > > > ABataev wrote:
> > > > > > > doru1004 wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > Do you really need to store the variable in the stack, is not 
> > > > > > > > > it enough just to check that the type of this variable is 
> > > > > > > > > BuiltinType::OMPIterator?
> > > > > > > > I'm happy to replace this if you think it will work. Is there 
> > > > > > > > an example somewhere in the code where I can get from the 
> > > > > > > > VarDecl to the build in type you mention?
> > > > > > > You have already a check 
> > > > > > > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> > > > > > >  you can you something similar for the variable
> > > > > > This didn't work and I had to revert to using the stack!
> > > > > Why?
> > > > I checked the output of the check and it was false when it should have 
> > > > been true! If you check the latest test that I added it showcases the 
> > > > source code and in the case of OpenMP 5.2 you can see that the message 
> > > > "only variable 'vvec' is allowed in map clauses of this 'omp declare 
> > > > mapper' directive" doesn't appear when a legal iteration variable is 
> > > > used.
> > > > If I used the check you suggested then the error message appears.
> > > > In the example you pasted the check is performed on a `Expr *`. In the 
> > > > case here, we only have VD which is a VarDecl.
> > > I am not sure how I can force it to have that type when it just doesn't. 
> > > Do you have any suggestions?
> > Did not get it. It still shall be of type builtintype::OMPIterator.
> The VD that we are checking for this builtin is coming from somewhere else in 
> the code, it is passed into the `Sema::DiagnoseUseOfDecl(` function. It's not 
> a VarDecl that is under the control of anything added in this patch.
This implementation is in line with the current checks for the declaration of 
the mapper variable. You store the declaration onto the stack so that you can 
compare it with the incoming VarDecl passed to the diagnose function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

ABataev wrote:
> doru1004 wrote:
> > doru1004 wrote:
> > > ABataev wrote:
> > > > doru1004 wrote:
> > > > > ABataev wrote:
> > > > > > doru1004 wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Do you really need to store the variable in the stack, is not 
> > > > > > > > it enough just to check that the type of this variable is 
> > > > > > > > BuiltinType::OMPIterator?
> > > > > > > I'm happy to replace this if you think it will work. Is there an 
> > > > > > > example somewhere in the code where I can get from the VarDecl to 
> > > > > > > the build in type you mention?
> > > > > > You have already a check 
> > > > > > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> > > > > >  you can you something similar for the variable
> > > > > This didn't work and I had to revert to using the stack!
> > > > Why?
> > > I checked the output of the check and it was false when it should have 
> > > been true! If you check the latest test that I added it showcases the 
> > > source code and in the case of OpenMP 5.2 you can see that the message 
> > > "only variable 'vvec' is allowed in map clauses of this 'omp declare 
> > > mapper' directive" doesn't appear when a legal iteration variable is used.
> > > If I used the check you suggested then the error message appears.
> > > In the example you pasted the check is performed on a `Expr *`. In the 
> > > case here, we only have VD which is a VarDecl.
> > I am not sure how I can force it to have that type when it just doesn't. Do 
> > you have any suggestions?
> Did not get it. It still shall be of type builtintype::OMPIterator.
The VD that we are checking for this builtin is coming from somewhere else in 
the code, it is passed into the `Sema::DiagnoseUseOfDecl(` function. It's not a 
VarDecl that is under the control of anything added in this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

doru1004 wrote:
> ABataev wrote:
> > doru1004 wrote:
> > > ABataev wrote:
> > > > doru1004 wrote:
> > > > > ABataev wrote:
> > > > > > Do you really need to store the variable in the stack, is not it 
> > > > > > enough just to check that the type of this variable is 
> > > > > > BuiltinType::OMPIterator?
> > > > > I'm happy to replace this if you think it will work. Is there an 
> > > > > example somewhere in the code where I can get from the VarDecl to the 
> > > > > build in type you mention?
> > > > You have already a check 
> > > > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> > > >  you can you something similar for the variable
> > > This didn't work and I had to revert to using the stack!
> > Why?
> I checked the output of the check and it was false when it should have been 
> true! If you check the latest test that I added it showcases the source code 
> and in the case of OpenMP 5.2 you can see that the message "only variable 
> 'vvec' is allowed in map clauses of this 'omp declare mapper' directive" 
> doesn't appear when a legal iteration variable is used.
> If I used the check you suggested then the error message appears.
> In the example you pasted the check is performed on a `Expr *`. In the case 
> here, we only have VD which is a VarDecl.
I am not sure how I can force it to have that type when it just doesn't. Do you 
have any suggestions?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

doru1004 wrote:
> ABataev wrote:
> > doru1004 wrote:
> > > ABataev wrote:
> > > > doru1004 wrote:
> > > > > ABataev wrote:
> > > > > > Do you really need to store the variable in the stack, is not it 
> > > > > > enough just to check that the type of this variable is 
> > > > > > BuiltinType::OMPIterator?
> > > > > I'm happy to replace this if you think it will work. Is there an 
> > > > > example somewhere in the code where I can get from the VarDecl to the 
> > > > > build in type you mention?
> > > > You have already a check 
> > > > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> > > >  you can you something similar for the variable
> > > This didn't work and I had to revert to using the stack!
> > Why?
> I checked the output of the check and it was false when it should have been 
> true! If you check the latest test that I added it showcases the source code 
> and in the case of OpenMP 5.2 you can see that the message "only variable 
> 'vvec' is allowed in map clauses of this 'omp declare mapper' directive" 
> doesn't appear when a legal iteration variable is used.
> If I used the check you suggested then the error message appears.
> In the example you pasted the check is performed on a `Expr *`. In the case 
> here, we only have VD which is a VarDecl.
Did not get it. It still shall be of type builtintype::OMPIterator.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

ABataev wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > doru1004 wrote:
> > > > ABataev wrote:
> > > > > Do you really need to store the variable in the stack, is not it 
> > > > > enough just to check that the type of this variable is 
> > > > > BuiltinType::OMPIterator?
> > > > I'm happy to replace this if you think it will work. Is there an 
> > > > example somewhere in the code where I can get from the VarDecl to the 
> > > > build in type you mention?
> > > You have already a check 
> > > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> > >  you can you something similar for the variable
> > This didn't work and I had to revert to using the stack!
> Why?
I checked the output of the check and it was false when it should have been 
true! If you check the latest test that I added it showcases the source code 
and in the case of OpenMP 5.2 you can see that the message "only variable 
'vvec' is allowed in map clauses of this 'omp declare mapper' directive" 
doesn't appear when a legal iteration variable is used.
If I used the check you suggested then the error message appears.
In the example you pasted the check is performed on a `Expr *`. In the case 
here, we only have VD which is a VarDecl.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

doru1004 wrote:
> ABataev wrote:
> > doru1004 wrote:
> > > ABataev wrote:
> > > > Do you really need to store the variable in the stack, is not it enough 
> > > > just to check that the type of this variable is 
> > > > BuiltinType::OMPIterator?
> > > I'm happy to replace this if you think it will work. Is there an example 
> > > somewhere in the code where I can get from the VarDecl to the build in 
> > > type you mention?
> > You have already a check 
> > IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator),
> >  you can you something similar for the variable
> This didn't work and I had to revert to using the stack!
Why?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

ABataev wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > Do you really need to store the variable in the stack, is not it enough 
> > > just to check that the type of this variable is BuiltinType::OMPIterator?
> > I'm happy to replace this if you think it will work. Is there an example 
> > somewhere in the code where I can get from the VarDecl to the build in type 
> > you mention?
> You have already a check 
> IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator), 
> you can you something similar for the variable
This didn't work and I had to revert to using the stack!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 490277.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_map_messages.cpp

Index: clang/test/OpenMP/target_map_messages.cpp
===
--- clang/test/OpenMP/target_map_messages.cpp
+++ clang/test/OpenMP/target_map_messages.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -verify=expected,lt50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=45 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=50 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51,omp,ge51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=51 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,ge50,ge51,ge52,omp,ge52-omp -fopenmp -fno-openmp-extensions -fopenmp-version=52 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -DCCODE -verify -fopenmp -fno-openmp-extensions -ferror-limit 300 -x c %s -Wno-openmp -Wuninitialized
 
 // -fopenmp-simd, -fno-openmp-extensions
@@ -158,23 +159,28 @@
 // expected-error@+1 {{use of undeclared identifier 'present'}}
 #pragma omp target map(present)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[1:2],f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f[1:2])
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[:],f)
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
@@ -191,11 +197,15 @@
 // lt51-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(present, present, tofrom: a)
 {}
+// ge52-omp-error@+5 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ompx-error@+3 {{same map type modifier has been specified more than once}}
 // ge51-omp-error@+2 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 2 {{incorrect map type modifier, expected one of: 

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

ABataev wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > Test cases for wrong variables in mappers?
> > You mean as part of the iterator ? like iterator(it = 0:UndefVar) ?
> I mean, you have a check that in mappers only iterator vars are allowed. Can 
> you add a check for this?
As far as I understand it the additional check I added to the existing mapper 
checks is needed because of the way the mapper check was written. The mapper 
checks looks at declarations and if a mapper clause exists then it assumes that 
the declaration must be coming from that mapper clause. This used to hold in 
the past since that was the only way you could have a declaration. This is not 
true anymore since we can now have declarations coming from both the mapper and 
the iterator modifier. I'll add a test to showcase this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

doru1004 wrote:
> ABataev wrote:
> > Test cases for wrong variables in mappers?
> You mean as part of the iterator ? like iterator(it = 0:UndefVar) ?
I mean, you have a check that in mappers only iterator vars are allowed. Can 
you add a check for this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

ABataev wrote:
> Test cases for wrong variables in mappers?
You mean as part of the iterator ? like iterator(it = 0:UndefVar) ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:5779-5780
 
+  /// Has iterator modifier
+  bool HasIteratorModifier = false;
+

It can be removed



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

Test cases for wrong variables in mappers?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 490188.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_map_messages.cpp

Index: clang/test/OpenMP/target_map_messages.cpp
===
--- clang/test/OpenMP/target_map_messages.cpp
+++ clang/test/OpenMP/target_map_messages.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -verify=expected,lt50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=45 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=50 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51,omp,ge51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=51 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,ge50,ge51,ge52,omp,ge52-omp -fopenmp -fno-openmp-extensions -fopenmp-version=52 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -DCCODE -verify -fopenmp -fno-openmp-extensions -ferror-limit 300 -x c %s -Wno-openmp -Wuninitialized
 
 // -fopenmp-simd, -fno-openmp-extensions
@@ -158,23 +159,28 @@
 // expected-error@+1 {{use of undeclared identifier 'present'}}
 #pragma omp target map(present)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[1:2],f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f[1:2])
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[:],f)
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
@@ -191,11 +197,15 @@
 // lt51-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(present, present, tofrom: a)
 {}
+// ge52-omp-error@+5 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ompx-error@+3 {{same map type modifier has been specified more than once}}
 // ge51-omp-error@+2 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma 

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

doru1004 wrote:
> ABataev wrote:
> > Do you really need to store the variable in the stack, is not it enough 
> > just to check that the type of this variable is BuiltinType::OMPIterator?
> I'm happy to replace this if you think it will work. Is there an example 
> somewhere in the code where I can get from the VarDecl to the build in type 
> you mention?
You have already a check 
IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator), 
you can you something similar for the variable


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:3474-3475
   bool parseMapperModifier(Sema::OpenMPVarListDataTy );
+  /// Parses the iterator modifier in map clause.
+  bool parseIteratorModifier(Sema::OpenMPVarListDataTy );
   /// Parses map-type-modifiers in map clause.

ABataev wrote:
> Where is it defined, cannot find it in the patch?
Leftover, removed it.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

ABataev wrote:
> Do you really need to store the variable in the stack, is not it enough just 
> to check that the type of this variable is BuiltinType::OMPIterator?
I'm happy to replace this if you think it will work. Is there an example 
somewhere in the code where I can get from the VarDecl to the build in type you 
mention?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 489942.
doru1004 marked 3 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_map_messages.cpp

Index: clang/test/OpenMP/target_map_messages.cpp
===
--- clang/test/OpenMP/target_map_messages.cpp
+++ clang/test/OpenMP/target_map_messages.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -verify=expected,lt50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=45 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=50 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51,omp,ge51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=51 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,ge50,ge51,ge52,omp,ge52-omp -fopenmp -fno-openmp-extensions -fopenmp-version=52 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -DCCODE -verify -fopenmp -fno-openmp-extensions -ferror-limit 300 -x c %s -Wno-openmp -Wuninitialized
 
 // -fopenmp-simd, -fno-openmp-extensions
@@ -158,23 +159,28 @@
 // expected-error@+1 {{use of undeclared identifier 'present'}}
 #pragma omp target map(present)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[1:2],f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f[1:2])
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[:],f)
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
@@ -191,11 +197,15 @@
 // lt51-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(present, present, tofrom: a)
 {}
+// ge52-omp-error@+5 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ompx-error@+3 {{same map type modifier has been specified more than once}}
 // ge51-omp-error@+2 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 2 {{incorrect map type modifier, expected one of: 

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:5869-5870
+if (IteratorModifier) {
+  HasIteratorModifier = true;
+  getTrailingObjects()[2 * varlist_size()] = IteratorModifier;
+}

Can we use just a nullptr as a mark that there is no associated iterator?



Comment at: clang/include/clang/Parse/Parser.h:3474-3475
   bool parseMapperModifier(Sema::OpenMPVarListDataTy );
+  /// Parses the iterator modifier in map clause.
+  bool parseIteratorModifier(Sema::OpenMPVarListDataTy );
   /// Parses map-type-modifiers in map clause.

Where is it defined, cannot find it in the patch?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) 
{
+  return IteratorVD == VD->getCanonicalDecl();
+});

Do you really need to store the variable in the stack, is not it enough just to 
check that the type of this variable is BuiltinType::OMPIterator?



Comment at: clang/test/OpenMP/target_map_messages.cpp:256-257
+// lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper'}}
+#pragma omp target map(iterator(it=0:10), tofrom:a)
+{}
 return;

Cases with incorrect parsing, wrong variables etc.?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 489917.
doru1004 marked 3 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_map_messages.cpp

Index: clang/test/OpenMP/target_map_messages.cpp
===
--- clang/test/OpenMP/target_map_messages.cpp
+++ clang/test/OpenMP/target_map_messages.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -verify=expected,lt50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=45 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=50 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51,omp,ge51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=51 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,ge50,ge51,ge52,omp,ge52-omp -fopenmp -fno-openmp-extensions -fopenmp-version=52 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -DCCODE -verify -fopenmp -fno-openmp-extensions -ferror-limit 300 -x c %s -Wno-openmp -Wuninitialized
 
 // -fopenmp-simd, -fno-openmp-extensions
@@ -158,23 +159,28 @@
 // expected-error@+1 {{use of undeclared identifier 'present'}}
 #pragma omp target map(present)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[1:2],f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f[1:2])
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[:],f)
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
@@ -191,11 +197,15 @@
 // lt51-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(present, present, tofrom: a)
 {}
+// ge52-omp-error@+5 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ompx-error@+3 {{same map type modifier has been specified more than once}}
 // ge51-omp-error@+2 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 2 {{incorrect 

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Erroneous test are required




Comment at: clang/include/clang/AST/OpenMPClause.h:310-314
+  /// Fetches list of variables associated with this clause.
+  Expr *getIteratorRef() {
+return (static_cast(this)
+->template getTrailingObjects())[2 * NumVars];
+  }

Why do we need it here, if it is specific to map clause?



Comment at: clang/include/clang/AST/OpenMPClause.h:317
+  /// Sets the list of variables for this clause.
+  void setIteratorRef(ArrayRef IL) {
+assert(IL.size() == 1 && "Number of iterator expressions must be 1");

Why ArrayRef if only one expression is expected?



Comment at: clang/include/clang/AST/OpenMPClause.h:5940-5942
+if (HasIteratorModifier)
+  return getIteratorRef();
+return nullptr;

Just `getIteratorRef()` should be enough.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1169
+for (auto *IteratorVD : Top->IteratorVarDecls)
+  if (IteratorVD == VD->getCanonicalDecl())
+return true;
+return false;

Use llvm::any_of()


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 489787.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_ast_print.cpp

Index: clang/test/OpenMP/target_ast_print.cpp
===
--- clang/test/OpenMP/target_ast_print.cpp
+++ clang/test/OpenMP/target_ast_print.cpp
@@ -1139,6 +1139,60 @@
 }
 #endif // OMP51
 
+#ifdef OMP52
+
+///==///
+// RUN: %clang_cc1 -DOMP52 -verify -fopenmp -fopenmp-version=52 -ast-print %s | FileCheck %s --check-prefix OMP52
+// RUN: %clang_cc1 -DOMP52 -fopenmp -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -DOMP52 -fopenmp -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s --check-prefix OMP52
+
+// RUN: %clang_cc1 -DOMP52 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s --check-prefix OMP52
+// RUN: %clang_cc1 -DOMP52 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -DOMP52 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s --check-prefix OMP52
+
+void foo() {}
+
+template 
+T tmain(T argc, T *argv) {
+  int N = 100;
+  int v[N];
+  #pragma omp target map(iterator(it = 0:N:2), to: v[it])
+  foo();
+  #pragma omp target map(iterator(it = 0:N:4), from: v[it])
+  foo();
+
+  return 0;
+}
+
+// OMP52: template  T tmain(T argc, T *argv) {
+// OMP52-NEXT: int N = 100;
+// OMP52-NEXT: int v[N];
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:N:2),to: v[it])
+// OMP52-NEXT: foo()
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:N:4),from: v[it])
+// OMP52-NEXT: foo()
+
+// OMP52-LABEL: int main(int argc, char **argv) {
+int main (int argc, char **argv) {
+  int i, j, a[20], always, close;
+// OMP52-NEXT: int i, j, a[20]
+#pragma omp target
+// OMP52-NEXT: #pragma omp target
+  foo();
+// OMP52-NEXT: foo();
+#pragma omp target map(iterator(it = 0:20:2), to: a[it])
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:20:2),to: a[it])
+  foo();
+// OMP52-NEXT: foo();
+#pragma omp target map(iterator(it = 0:20:4), from: a[it])
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:20:4),from: a[it])
+foo();
+// OMP52-NEXT: foo();
+
+  return tmain(argc, ) + tmain(argv[0][0], argv[0]);
+}
+#endif // OMP52
+
 #ifdef OMPX
 
 // RUN: %clang_cc1 -DOMPX -verify -fopenmp -fopenmp-extensions -ast-print %s | FileCheck %s --check-prefix=OMPX
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6792,6 +6792,8 @@
   for (unsigned I = 0; I < NumberOfOMPMapClauseModifiers; ++I) {
 Record.push_back(C->getMapTypeModifier(I));
 Record.AddSourceLocation(C->getMapTypeModifierLoc(I));
+if (C->getMapTypeModifier(I) == OMPC_MAP_MODIFIER_iterator)
+  Record.AddStmt(C->getIteratorModifier());
   }
   Record.AddNestedNameSpecifierLoc(C->getMapperQualifierLoc());
   Record.AddDeclarationNameInfo(C->getMapperIdInfo());
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -10773,6 +10773,8 @@
 C->setMapTypeModifier(
 I, static_cast(Record.readInt()));
 C->setMapTypeModifierLoc(I, Record.readSourceLocation());
+if (C->getMapTypeModifier(I) == OMPC_MAP_MODIFIER_iterator)
+  C->setIteratorModifier(Record.readSubExpr());
   }
   C->setMapperQualifierLoc(Record.readNestedNameSpecifierLoc());
   C->setMapperIdInfo(Record.readDeclarationNameInfo());
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -1988,15 +1988,16 @@
   /// By default, performs semantic analysis to build the new OpenMP clause.
   /// Subclasses may override this routine to provide different behavior.
   OMPClause *RebuildOMPMapClause(
-  ArrayRef MapTypeModifiers,
+  Expr *IteratorModifier, ArrayRef MapTypeModifiers,
   ArrayRef MapTypeModifiersLoc,
   CXXScopeSpec 

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:5780
+  /// Iterator modifier
+  Expr *IteratorModifierExpr = nullptr;
+

We're using tail allocation for such objects.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 created this revision.
doru1004 added reviewers: jdoerfert, ABataev, carlo.bertolli, ronl, 
gregrodgers, jhuber6.
doru1004 added a project: OpenMP.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
doru1004 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch adds parse and sema support for iterator map modifier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_ast_print.cpp

Index: clang/test/OpenMP/target_ast_print.cpp
===
--- clang/test/OpenMP/target_ast_print.cpp
+++ clang/test/OpenMP/target_ast_print.cpp
@@ -1139,6 +1139,60 @@
 }
 #endif // OMP51
 
+#ifdef OMP52
+
+///==///
+// RUN: %clang_cc1 -DOMP52 -verify -fopenmp -fopenmp-version=52 -ast-print %s | FileCheck %s --check-prefix OMP52
+// RUN: %clang_cc1 -DOMP52 -fopenmp -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -DOMP52 -fopenmp -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s --check-prefix OMP52
+
+// RUN: %clang_cc1 -DOMP52 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s --check-prefix OMP52
+// RUN: %clang_cc1 -DOMP52 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -DOMP52 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s --check-prefix OMP52
+
+void foo() {}
+
+template 
+T tmain(T argc, T *argv) {
+  int N = 100;
+  int v[N];
+  #pragma omp target map(iterator(it = 0:N:2), to: v[it])
+  foo();
+  #pragma omp target map(iterator(it = 0:N:4), from: v[it])
+  foo();
+
+  return 0;
+}
+
+// OMP52: template  T tmain(T argc, T *argv) {
+// OMP52-NEXT: int N = 100;
+// OMP52-NEXT: int v[N];
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:N:2),to: v[it])
+// OMP52-NEXT: foo()
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:N:4),from: v[it])
+// OMP52-NEXT: foo()
+
+// OMP52-LABEL: int main(int argc, char **argv) {
+int main (int argc, char **argv) {
+  int i, j, a[20], always, close;
+// OMP52-NEXT: int i, j, a[20]
+#pragma omp target
+// OMP52-NEXT: #pragma omp target
+  foo();
+// OMP52-NEXT: foo();
+#pragma omp target map(iterator(it = 0:20:2), to: a[it])
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:20:2),to: a[it])
+  foo();
+// OMP52-NEXT: foo();
+#pragma omp target map(iterator(it = 0:20:4), from: a[it])
+// OMP52-NEXT: #pragma omp target map(iterator(int it = 0:20:4),from: a[it])
+foo();
+// OMP52-NEXT: foo();
+
+  return tmain(argc, ) + tmain(argv[0][0], argv[0]);
+}
+#endif // OMP52
+
 #ifdef OMPX
 
 // RUN: %clang_cc1 -DOMPX -verify -fopenmp -fopenmp-extensions -ast-print %s | FileCheck %s --check-prefix=OMPX
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6792,6 +6792,8 @@
   for (unsigned I = 0; I < NumberOfOMPMapClauseModifiers; ++I) {
 Record.push_back(C->getMapTypeModifier(I));
 Record.AddSourceLocation(C->getMapTypeModifierLoc(I));
+if (C->getMapTypeModifier(I) == OMPC_MAP_MODIFIER_iterator)
+  Record.AddStmt(C->getIteratorModifier());
   }
   Record.AddNestedNameSpecifierLoc(C->getMapperQualifierLoc());
   Record.AddDeclarationNameInfo(C->getMapperIdInfo());
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -10773,6 +10773,8 @@
 C->setMapTypeModifier(
 I, static_cast(Record.readInt()));
 C->setMapTypeModifierLoc(I, Record.readSourceLocation());
+if (C->getMapTypeModifier(I) == OMPC_MAP_MODIFIER_iterator)
+  C->setIteratorModifier(Record.readSubExpr());
   }
   C->setMapperQualifierLoc(Record.readNestedNameSpecifierLoc());
   C->setMapperIdInfo(Record.readDeclarationNameInfo());
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -1988,15 +1988,16 @@