https://github.com/zahiraam closed
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Meinersbur approved this pull request.
LGTM, thanks.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mjklemm approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zahiraam wrote:
@mjklemm @Meinersbur any more things to add? thanks.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,46 +1,89 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45
-fopenmp -fopenmp-version=45 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0
-verify=expected,omp5,host5 -fopenmp -fopenmp-version=50
@@ -1,46 +1,89 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45
-fopenmp -fopenmp-version=45 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0
-verify=expected,omp5,host5 -fopenmp -fopenmp-version=50
@@ -254,4 +256,3 @@ int main(int argc, char **argv) {
foomain(argc,argv); // expected-note {{n instantiation of function
template specialization 'foomain' requested here}}
return 0;
}
-
Meinersbur wrote:
[nit] unrelated change
https://github.com/llvm/llv
@@ -1,46 +1,89 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45
-fopenmp -fopenmp-version=45 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0
-verify=expected,omp5,host5 -fopenmp -fopenmp-version=50
https://github.com/saiislam approved this pull request.
Thank you for sticking up with this patch @zahiraam.
It mostly looks ok to me, but please wait for @mjklemm and @Meinersbur reviews.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commit
@@ -205,77 +262,134 @@ namespace {
}
#pragma omp end declare target
-#pragma omp declare target link(S) // expected-error {{'S' used in declare
target directive is not a variable or a function name}}
+// expected-error@+1 {{'S' used in declare target directive is not a variab
https://github.com/Meinersbur commented:
Some nitpicks left. I will LGTM this after you applied them and nobody else has
remarks.
Converting all the lines to `@+` syntax would not have been necessary, only
those you needed to change. I think it has become a lot more maintainable now.
For pref
https://github.com/Meinersbur edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -68,15 +70,21 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
@@ -205,77 +262,134 @@ namespace {
}
#pragma omp end declare target
-#pragma omp declare target link(S) // expected-error {{'S' used in declare
target directive is not a variable or a function name}}
+// expected-error@+1 {{'S' used in declare target directive is not a variab
@@ -68,15 +70,21 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
@@ -124,4 +124,3 @@ target *S1 = &S;
// CHECK-NEXT: #pragma omp declare target
// CHECK-NEXT: target *S1 = &S;
// CHECK-NEXT: #pragma omp end declare target
-
Meinersbur wrote:
[nit] unrelated change
https://github.com/llvm/llvm-project/pull/122108
__
zahiraam wrote:
ping @mjklemm.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zahiraam wrote:
> Yes, I think that would be helpful. Also, avoid duplicating lines with
> multiple `-verify=` prefixes.
I have made an attempt to edit the 2 LIT tests. I had a hard time finding good
names for the `verify` prefixes. Please don't hesitate to suggest better ones.
We could also u
Meinersbur wrote:
Yes, I think that would be helpful. Also, avoid duplicating lines with multiple
`-verify=` prefixes.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-
https://github.com/Meinersbur edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Meinersbur edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zahiraam wrote:
> I don't think it is necessary to test each value of `-fopenmp-version=`
> separately, but don't mind either as long it doesn't affect maintanability.
>
> * Instead of repeating each clang diagnositic, consider passing multiple
> values to `-verify`, e.g. `-verify=omp51,omp60`
https://github.com/Meinersbur commented:
I don't think it is necessary to test each value of `-fopenmp-version=`
separately, but don't mind either as long it doesn't affect maintanability.
* Instead of repeating each clang diagnositic, consider passing multiple
values to `-verify`, e.g. `-ve
@@ -2,9 +2,16 @@
// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -fopenmp-version=51 -emit-pch -o
%t %s
// RUN: %clang_cc1 -fopenmp -std=c++11 -fopenmp-version=51 -include-pch %t
-verify %s -ast-print | FileCheck %s
Meinersbur wrote:
[nit] Consistency: Either
https://github.com/Meinersbur edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -12,11 +18,16 @@
// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51
-fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp52
-fopenmp -fopenmp-version=52 -DVERBOSE_MODE=1
@@ -68,15 +70,15 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
zahiraam wrote:
> Thanks for revising the PR. I have added some comments.
>
> To unblock #119891, wouldn't it be sufficient that `-fopenmp-version=60`
> becomes a valid value? Do we need to modify all these tests to test only this
> flag?
Yes it's sufficient and that's what's done in #119891.
https://github.com/zahiraam edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zahiraam wrote:
> Please address @saiislam's questions. I also think that the title and
> description of the PR should reflect that this PR not only introduces the 6.0
> flag, but also moves several of the tests to test against 5.1 and 6.0.
I responded to the question.
The changes in the tests
Meinersbur wrote:
> In #119891 we are introducing the support for `#pragma omp stripe` which is
> an OMP6.0 feature (see [#119891
> (files)](https://github.com/llvm/llvm-project/pull/119891/files#r1896944800)).
> This PR changes the default option to 6.0.
Summary (which usually is the basis f
https://github.com/mjklemm requested changes to this pull request.
Please address @saiislam's questions. I also think that the title and
description of the PR should reflect that this PR not only introduces the 6.0
flag, but also moves several of the tests to test against 5.1 and 6.0.
https:
zahiraam wrote:
> Please resolve open feedback items before the next round of reviews. Thanks.
Sorry I might have been ahead of myself!
I think this time I might have gotten it.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing
@@ -19,6 +19,8 @@ typedef void *omp_depend_t;
void foo() {}
void tmainc(){
omp_depend_t obj;
+ int omp_all_memory;
zahiraam wrote:
Removed.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits maili
mjklemm wrote:
Please resolve open feedback items before the next round of reviews. Thanks.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
zahiraam wrote:
@mjklemm, @saiislam can you please review this? Thanks.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -68,15 +70,15 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
@@ -68,15 +70,15 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
https://github.com/saiislam commented:
Thanks for the revising the PR. I have added some comments.
To unblock #119891, wouldn't it be sufficient that `-fopenmp-version=60`
becomes a valid value? Do we need to modify all these tests to test only this
flag?
https://github.com/llvm/llvm-project/
@@ -63,22 +63,22 @@ class VV {
// CHECK-NEXT: int add(int a, int b) __attribute__((cold)){
// CHECK-NEXT: return a + b;
// CHECK-NEXT: }
- #pragma omp declare simd uniform(this, a) linear(val(b): a)
+ #pragma omp declare simd uniform(this, a) linear(b: a)
--
https://github.com/saiislam edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -25,16 +36,16 @@ __thread int t; // expected-note {{defined as threadprivate
or thread local}}
void f();
#pragma omp end declare target shared(a) // expected-warning {{extra tokens at
the end of '#pragma omp end declare target' are ignored}}
-#pragma omp declare target ma
@@ -4,19 +4,19 @@
// RUN: %clang_cc1 -verify -std=c++11 -fopenmp -ast-print %s
-Wno-openmp-mapping -DOMP5 | FileCheck %s --check-prefix=CHECK
--check-prefix=OMP50
// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s -DOMP5
// RUN: %clang_cc1 -fopenmp -std=c++11 -i
@@ -68,15 +70,15 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
https://github.com/saiislam edited
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -69,11 +80,11 @@ void c();
void func() {} // expected-note {{'func' defined here}}
-#pragma omp declare target link(func) allocate(a) // expected-error {{function
name is not allowed in 'link' clause}} omp45-error {{unexpected 'allocate'
clause, only 'to' or 'link' claus
@@ -1,10 +1,10 @@
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -emit-llvm
-o - %s | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -triple x86_64-apple-darwin10 -x c++ -std=c++11
-emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -triple x86_64-apple-darwin10 -std=
@@ -19,6 +19,8 @@ typedef void *omp_depend_t;
void foo() {}
void tmainc(){
omp_depend_t obj;
+ int omp_all_memory;
saiislam wrote:
Do we need this new variable?
https://github.com/llvm/llvm-project/pull/122108
@@ -68,15 +70,15 @@ int fun(int arg) {
{}
#pragma omp target map(mapper(aa :vv) //
expected-error {{use of undeclared identifier 'aa'}} expected-error {{expected
')'}} expected-error {{call to undeclared function 'mapper'}} expected-note
zahiraam wrote:
@mjklemm and @saiislam would you mind taking a look at this please? Thanks.
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
https://github.com/zahiraam ready_for_review
https://github.com/llvm/llvm-project/pull/122108
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
51 matches
Mail list logo
