| Message ID | 3ec10d98-7dd6-4d25-9df5-7856cd87649f@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix tests for gomp | expand |
On Wed, Dec 13, 2023 at 10:43:16AM +0000, Andre Vieira (lists) wrote: > Hi, > > Apologies for the delay and this mixup. I need to do something different > > This is to fix testisms initially introduced by: > commit f5fc001a84a7dbb942a6252b3162dd38b4aae311 > Author: Andre Vieira <andre.simoesdiasvieira@arm.com> > Date: Mon Dec 11 14:24:41 2023 +0000 > > aarch64: enable mixed-types for aarch64 simdclones > > gcc/testsuite/ChangeLog: > > * gcc.dg/gomp/pr87887-1.c: Fixed test. > * gcc.dg/gomp/pr89246-1.c: Likewise. > * gcc.dg/gomp/simd-clones-2.c: Likewise. > > libgomp/ChangeLog: > > * testsuite/libgomp.c/declare-variant-1.c: Fixed test. > * testsuite/libgomp.fortran/declare-simd-1.f90: Likewise. > > OK for trunk? I was intending to commit as obvious, but jakub had made a > comment about declare-simd-1.f90 so I thought it might be worth just sending > it up to the mailing list first. > --- a/libgomp/testsuite/libgomp.c/declare-variant-1.c > +++ b/libgomp/testsuite/libgomp.c/declare-variant-1.c > @@ -40,16 +40,17 @@ f04 (int a) > int > test1 (int x) > { > - /* At gimplification time, we can't decide yet which function to call. */ > - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ > + /* At gimplification time, we can't decide yet which function to call for > + x86_64 targets, given the f01 variant. */ > + /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" { target x86_64-*-* } } } */ > /* After simd clones are created, the original non-clone test1 shall > call f03 (score 6), the sse2/avx/avx2 clones too, but avx512f clones > shall call f01 with score 8. */ > /* { dg-final { scan-ltrans-tree-dump-not "f04 \\\(x" "optimized" } } */ > - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } } */ > - /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } } */ > - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } } */ > - /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } } */ > + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } */ > + /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } */ > + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } */ > + /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } */ The changes in this test look all wrong. The differences are i?86-*-* x86_64-*-* (which can support avx512f isa) vs. other targets (which can't). So, there is nothing aarch64 specific in there and { target x86_64-*-* } is also incorrect. It should be simply { target i?86-*-* x86_64-*-* } vs. { target { ! { i?86-*-* x86_64-*-* } } } (never sure about the ! syntaxes). The other changes LGTM. Jakub
On Wed, Dec 13, 2023 at 11:55:52AM +0100, Jakub Jelinek wrote: > On Wed, Dec 13, 2023 at 10:43:16AM +0000, Andre Vieira (lists) wrote: > > --- a/libgomp/testsuite/libgomp.c/declare-variant-1.c > > +++ b/libgomp/testsuite/libgomp.c/declare-variant-1.c > > @@ -40,16 +40,17 @@ f04 (int a) > > int > > test1 (int x) > > { > > - /* At gimplification time, we can't decide yet which function to call. */ > > - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ > > + /* At gimplification time, we can't decide yet which function to call for > > + x86_64 targets, given the f01 variant. */ > > + /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" { target x86_64-*-* } } } */ > > /* After simd clones are created, the original non-clone test1 shall > > call f03 (score 6), the sse2/avx/avx2 clones too, but avx512f clones > > shall call f01 with score 8. */ > > /* { dg-final { scan-ltrans-tree-dump-not "f04 \\\(x" "optimized" } } */ > > - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } } */ > > - /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } } */ > > - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } } */ > > - /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } } */ > > + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } */ > > + /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } */ > > + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } */ > > + /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } */ > > The changes in this test look all wrong. The differences are > i?86-*-* x86_64-*-* (which can support avx512f isa) vs. other targets (which > can't). > So, there is nothing aarch64 specific in there and { target x86_64-*-* } > is also incorrect. It should be simply > { target i?86-*-* x86_64-*-* } > vs. > { target { ! { i?86-*-* x86_64-*-* } } } > (never sure about the ! syntaxes). Or even better just make the whole test i?86-*-* x86_64-*-* specific. It is really testing the details how many clones are created and what is called in them, so nothing that applies to other architectures and how many clones those have will differ, but what exactly will be called in there will be the same. Jakub
On 13/12/2023 10:55, Jakub Jelinek wrote: > On Wed, Dec 13, 2023 at 10:43:16AM +0000, Andre Vieira (lists) wrote: >> Hi, >> >> Apologies for the delay and this mixup. I need to do something different >> >> This is to fix testisms initially introduced by: >> commit f5fc001a84a7dbb942a6252b3162dd38b4aae311 >> Author: Andre Vieira <andre.simoesdiasvieira@arm.com> >> Date: Mon Dec 11 14:24:41 2023 +0000 >> >> aarch64: enable mixed-types for aarch64 simdclones >> >> gcc/testsuite/ChangeLog: >> >> * gcc.dg/gomp/pr87887-1.c: Fixed test. >> * gcc.dg/gomp/pr89246-1.c: Likewise. >> * gcc.dg/gomp/simd-clones-2.c: Likewise. >> >> libgomp/ChangeLog: >> >> * testsuite/libgomp.c/declare-variant-1.c: Fixed test. >> * testsuite/libgomp.fortran/declare-simd-1.f90: Likewise. >> >> OK for trunk? I was intending to commit as obvious, but jakub had made a >> comment about declare-simd-1.f90 so I thought it might be worth just sending >> it up to the mailing list first. > >> --- a/libgomp/testsuite/libgomp.c/declare-variant-1.c >> +++ b/libgomp/testsuite/libgomp.c/declare-variant-1.c >> @@ -40,16 +40,17 @@ f04 (int a) >> int >> test1 (int x) >> { >> - /* At gimplification time, we can't decide yet which function to call. */ >> - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ >> + /* At gimplification time, we can't decide yet which function to call for >> + x86_64 targets, given the f01 variant. */ >> + /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" { target x86_64-*-* } } } */ >> /* After simd clones are created, the original non-clone test1 shall >> call f03 (score 6), the sse2/avx/avx2 clones too, but avx512f clones >> shall call f01 with score 8. */ >> /* { dg-final { scan-ltrans-tree-dump-not "f04 \\\(x" "optimized" } } */ >> - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } } */ >> - /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } } */ >> - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } } */ >> - /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } } */ >> + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } */ >> + /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } */ >> + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } */ >> + /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } */ > > The changes in this test look all wrong. The differences are > i?86-*-* x86_64-*-* (which can support avx512f isa) vs. other targets (which > can't). > So, there is nothing aarch64 specific in there and { target x86_64-*-* } > is also incorrect. It should be simply > { target i?86-*-* x86_64-*-* } > vs. > { target { ! { i?86-*-* x86_64-*-* } } } > (never sure about the ! syntaxes). > Hmm.... I think I understand what you are saying, but I'm not sure I agree. So before I enabled simdclone testing for aarch64, this test had no target selectors. So it checked the same for 'all simdclone test targets'. Which seem to be x86 and amdgcn: @@ -4321,7 +4321,8 @@ proc check_effective_target_vect_simd_clones { } { return [check_cached_effective_target_indexed vect_simd_clones { expr { (([istarget i?86-*-*] || [istarget x86_64-*-*]) && [check_effective_target_avx512f]) || [istarget amdgcn-*-*] || [istarget aarch64*-*-*] }}] } I haven't checked what amdgcn does with this test, but I'd have to assume they were passing before? Though I'm not sure how amdgcn would pass the original: - /* At gimplification time, we can't decide yet which function to call. */ - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ I've added Andrew to the mail to see if he can comment on that. Either way I'd suggest we either add scan's per target with the expected value or stick with my original change of aarch64 vs non-aarch64 as I think that would better reflect the changes of enabling this for aarch64 where it wasn't ran before.
On Wed, Dec 13, 2023 at 11:03:50AM +0000, Andre Vieira (lists) wrote: > Hmm.... I think I understand what you are saying, but I'm not sure I agree. > So before I enabled simdclone testing for aarch64, this test had no target > selectors. So it checked the same for 'all simdclone test targets'. Which > seem to be x86 and amdgcn: > > @@ -4321,7 +4321,8 @@ proc check_effective_target_vect_simd_clones { } { > return [check_cached_effective_target_indexed vect_simd_clones { > expr { (([istarget i?86-*-*] || [istarget x86_64-*-*]) > && [check_effective_target_avx512f]) > || [istarget amdgcn-*-*] > || [istarget aarch64*-*-*] }}] > } > > I haven't checked what amdgcn does with this test, but I'd have to assume > they were passing before? Though I'm not sure how amdgcn would pass the > original: > - /* At gimplification time, we can't decide yet which function to call. > */ > - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ It can't really pass there. amdgcn certainly doesn't create 4 different simd clones where one has avx512f isa and others don't. gcn creates just one simd clone with simdlen 64 and that clone will never support avx512f isa and we know that already at gimplification time. Jakub
Hi! On 2023-12-13T12:09:14+0100, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Dec 13, 2023 at 11:03:50AM +0000, Andre Vieira (lists) wrote: >> Hmm.... I think I understand what you are saying, but I'm not sure I agree. >> So before I enabled simdclone testing for aarch64, this test had no target >> selectors. So it checked the same for 'all simdclone test targets'. Which >> seem to be x86 and amdgcn: >> >> @@ -4321,7 +4321,8 @@ proc check_effective_target_vect_simd_clones { } { >> return [check_cached_effective_target_indexed vect_simd_clones { >> expr { (([istarget i?86-*-*] || [istarget x86_64-*-*]) >> && [check_effective_target_avx512f]) >> || [istarget amdgcn-*-*] >> || [istarget aarch64*-*-*] }}] >> } >> >> I haven't checked what amdgcn does with this test, but I'd have to assume >> they were passing before? Though I'm not sure how amdgcn would pass the >> original: >> --- a/libgomp/testsuite/libgomp.c/declare-variant-1.c >> +++ b/libgomp/testsuite/libgomp.c/declare-variant-1.c >> - /* At gimplification time, we can't decide yet which function to call. */ >> - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ > > It can't really pass there. amdgcn certainly doesn't create 4 different > simd clones where one has avx512f isa and others don't. > gcn creates just one simd clone with simdlen 64 and that clone will never > support avx512f isa and we know that already at gimplification time. For GCN target (and likewise, nvptx target) configurations, libgomp test cases currently are a total mess -- the reason being that those target configurations actually (largely) implement GCN or nvptx *offloading* configuration functionality: they lower OMP constructs and implement libgomp functions in a way that (largely) assumes that they're *offloading* instead of *target* configurations, and therefore things go horribly wrong. (This certainly is something worth fixing, but...) Therefore, currently, GCN or nvptx *target* configuration's 'check-target-libgomp' currently doesn't really have any value, and certainly isn't maintained in any way. Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
diff --git a/gcc/testsuite/gcc.dg/gomp/pr87887-1.c b/gcc/testsuite/gcc.dg/gomp/pr87887-1.c index 281898300c7794d862e62c70a83a33d5aaa8f89e..8b04ffd0809be4e6f5ab97c2e32e800edffbee4f 100644 --- a/gcc/testsuite/gcc.dg/gomp/pr87887-1.c +++ b/gcc/testsuite/gcc.dg/gomp/pr87887-1.c @@ -10,7 +10,6 @@ foo (int x) { return (struct S) { x }; } -/* { dg-warning "unsupported return type ‘struct S’ for ‘simd’ functions" "" { target aarch64*-*-* } .-4 } */ #pragma omp declare simd int @@ -18,7 +17,6 @@ bar (struct S x) { return x.n; } -/* { dg-warning "unsupported argument type ‘struct S’ for ‘simd’ functions" "" { target aarch64*-*-* } .-4 } */ #pragma omp declare simd uniform (x) int diff --git a/gcc/testsuite/gcc.dg/gomp/pr89246-1.c b/gcc/testsuite/gcc.dg/gomp/pr89246-1.c index 4a0fd74f0639b2832dcb9101e006d127568fbcbd..dfe629c1c6a51624cd94878c638606220cfe94eb 100644 --- a/gcc/testsuite/gcc.dg/gomp/pr89246-1.c +++ b/gcc/testsuite/gcc.dg/gomp/pr89246-1.c @@ -8,7 +8,6 @@ int foo (__int128 x) { return x; } -/* { dg-warning "unsupported argument type ‘__int128’ for ‘simd’ functions" "" { target aarch64*-*-* } .-4 } */ #pragma omp declare simd extern int bar (int x); diff --git a/gcc/testsuite/gcc.dg/gomp/simd-clones-2.c b/gcc/testsuite/gcc.dg/gomp/simd-clones-2.c index f12244054bd46fa10e51cc3a688c4cf683689994..354078acd9f3073b8400621a0e7149aee571594b 100644 --- a/gcc/testsuite/gcc.dg/gomp/simd-clones-2.c +++ b/gcc/testsuite/gcc.dg/gomp/simd-clones-2.c @@ -19,7 +19,6 @@ float setArray(float *a, float x, int k) /* { dg-final { scan-tree-dump "_ZGVnN2ua32vl_setArray" "optimized" { target aarch64*-*-* } } } */ /* { dg-final { scan-tree-dump "_ZGVnN4ua32vl_setArray" "optimized" { target aarch64*-*-* } } } */ /* { dg-final { scan-tree-dump "_ZGVnN2vvva32_addit" "optimized" { target aarch64*-*-* } } } */ -/* { dg-final { scan-tree-dump "_ZGVnN4vvva32_addit" "optimized" { target aarch64*-*-* } } } */ /* { dg-final { scan-tree-dump "_ZGVnM2vl66u_addit" "optimized" { target aarch64*-*-* } } } */ /* { dg-final { scan-tree-dump "_ZGVnM4vl66u_addit" "optimized" { target aarch64*-*-* } } } */ diff --git a/libgomp/testsuite/libgomp.c/declare-variant-1.c b/libgomp/testsuite/libgomp.c/declare-variant-1.c index 6129f23a0f80585246957022d63608dc3a68f1ff..790e9374054fe3e0ae609796640ff295b61e8389 100644 --- a/libgomp/testsuite/libgomp.c/declare-variant-1.c +++ b/libgomp/testsuite/libgomp.c/declare-variant-1.c @@ -40,16 +40,17 @@ f04 (int a) int test1 (int x) { - /* At gimplification time, we can't decide yet which function to call. */ - /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" } } */ + /* At gimplification time, we can't decide yet which function to call for + x86_64 targets, given the f01 variant. */ + /* { dg-final { scan-tree-dump-times "f04 \\\(x" 2 "gimple" { target x86_64-*-* } } } */ /* After simd clones are created, the original non-clone test1 shall call f03 (score 6), the sse2/avx/avx2 clones too, but avx512f clones shall call f01 with score 8. */ /* { dg-final { scan-ltrans-tree-dump-not "f04 \\\(x" "optimized" } } */ - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } } */ - /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } } */ - /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } } */ - /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } } */ + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 14 "optimized" { target { !aarch64*-*-* } } } } */ + /* { dg-final { scan-ltrans-tree-dump-times "f01 \\\(x" 4 "optimized" { target { !aarch64*-*-* } } } } */ + /* { dg-final { scan-ltrans-tree-dump-times "f03 \\\(x" 10 "optimized" { target { aarch64*-*-* } } } } */ + /* { dg-final { scan-ltrans-tree-dump-not "f01 \\\(x" "optimized" { target { aarch64*-*-* } } } } */ int a = f04 (x); int b = f04 (x); return a + b; diff --git a/libgomp/testsuite/libgomp.fortran/declare-simd-1.f90 b/libgomp/testsuite/libgomp.fortran/declare-simd-1.f90 index 9d4452459aa76dacc2f3be2dd78ccd28b04fc379..591915db3ad8541c5724e160968f7e8bdf7a639b 100644 --- a/libgomp/testsuite/libgomp.fortran/declare-simd-1.f90 +++ b/libgomp/testsuite/libgomp.fortran/declare-simd-1.f90 @@ -1,5 +1,7 @@ ! { dg-do run { target vect_simd_clones } } -! { dg-options "-fno-inline -cpp -D__aarch64__" } +! { dg-options "-fno-inline" } +! { dg-additional-options "-cpp -DSIMDLEN_BASE=8" { target { ! aarch64*-*-* } } } +! { dg-additional-options "-cpp -DSIMDLEN_BASE=4" { target aarch64*-*-* } } ! { dg-additional-options "-msse2" { target sse2_runtime } } ! { dg-additional-options "-mavx" { target avx_runtime } } @@ -75,11 +77,7 @@ end module declare_simd_1_mod end do contains function baz (x, y, z) -#ifdef __aarch64__ - !$omp declare simd (baz) simdlen (4) uniform (x, y) -#else - !$omp declare simd (baz) simdlen (8) uniform (x, y) -#endif + !$omp declare simd (baz) simdlen (SIMDLEN_BASE) uniform (x, y) !$omp declare simd (baz) integer, value :: y real, value :: z @@ -94,10 +92,6 @@ function bar (a, b, c) real :: bar double precision, value :: a !$omp declare simd (bar) -#ifdef __aarch64__ - !$omp declare simd (bar) simdlen (2) linear (b : 2) -#else - !$omp declare simd (bar) simdlen (4) linear (b : 2) -#endif + !$omp declare simd (bar) simdlen (SIMDLEN_BASE / 2) linear (b : 2) bar = a + b * c end function bar
Hi, Apologies for the delay and this mixup. I need to do something different This is to fix testisms initially introduced by: commit f5fc001a84a7dbb942a6252b3162dd38b4aae311 Author: Andre Vieira <andre.simoesdiasvieira@arm.com> Date: Mon Dec 11 14:24:41 2023 +0000 aarch64: enable mixed-types for aarch64 simdclones gcc/testsuite/ChangeLog: * gcc.dg/gomp/pr87887-1.c: Fixed test. * gcc.dg/gomp/pr89246-1.c: Likewise. * gcc.dg/gomp/simd-clones-2.c: Likewise. libgomp/ChangeLog: * testsuite/libgomp.c/declare-variant-1.c: Fixed test. * testsuite/libgomp.fortran/declare-simd-1.f90: Likewise. OK for trunk? I was intending to commit as obvious, but jakub had made a comment about declare-simd-1.f90 so I thought it might be worth just sending it up to the mailing list first. Kind regards, Andre