| Message ID | 20210313203406.GP3748@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Fix up aarch64_simd_clone_compute_vecsize_and_simdlen [PR99542] | expand |
Hi Jakub, > -----Original Message----- > From: Jakub Jelinek <jakub@redhat.com> > Sent: 13 March 2021 20:34 > To: Richard Sandiford <Richard.Sandiford@arm.com>; Richard Earnshaw > <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> > Cc: gcc-patches@gcc.gnu.org > Subject: [PATCH] aarch64: Fix up > aarch64_simd_clone_compute_vecsize_and_simdlen [PR99542] > > Hi! > > As the patch shows, there are several bugs in > aarch64_simd_clone_compute_vecsize_and_simdlen. > One is that unlike for function declarations that aren't definitions > it completely ignores argument types. Such decls don't have > DECL_ARGUMENTS, > but we can walk TYPE_ARG_TYPES instead, like the i386 backend does or like > the simd cloning code in the middle end does too. > > Another problem is that it checks types of uniform arguments. That is > unnecessary, uniform arguments are passed the way it normally is, it is > a scalar argument rather than vector, so there is no reason not to support > uniform argument of different size, or long double, structure etc. > > Fixed thusly, bootstrapped/regtested on aarch64-linux, ok for trunk? I don't have much experience in this part of GCC and I think you're best placed to make a judgement on the implementation of this hook. It's okay from my perspective (it doesn't look like it would break any SVE logic), but if you'd like to wait for a review from Richard I won't object. Thanks, Kyrill > > 2021-03-13 Jakub Jelinek <jakub@redhat.com> > > PR target/99542 > * config/aarch64/aarch64.c > (aarch64_simd_clone_compute_vecsize_and_simdlen): If not a > function > definition, walk TYPE_ARG_TYPES list if non-NULL for argument > types > instead of DECL_ARGUMENTS. Ignore types for uniform arguments. > > * gcc.dg/gomp/pr99542.c: New test. > * gcc.dg/gomp/pr59669-2.c (bar): Don't expect a warning on > aarch64. > * gcc.dg/gomp/simd-clones-2.c (setArray): Likewise. > * g++.dg/vect/simd-clone-7.cc (bar): Likewise. > * g++.dg/gomp/declare-simd-1.C (f37): Expect a different warning > on aarch64. > * gcc.dg/declare-simd.c (fn2): Expect a new warning on aarch64. > > --- gcc/config/aarch64/aarch64.c.jj 2021-03-12 19:01:48.672296344 > +0100 > +++ gcc/config/aarch64/aarch64.c 2021-03-13 09:16:42.585045538 > +0100 > @@ -23412,11 +23412,17 @@ > aarch64_simd_clone_compute_vecsize_and_s > return 0; > } > > - for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t)) > + int i; > + tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); > + bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE); > + > + for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i > = 0; > + t && t != void_list_node; t = TREE_CHAIN (t), i++) > { > - arg_type = TREE_TYPE (t); > + tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t); > > - if (!currently_supported_simd_type (arg_type, base_type)) > + if (clonei->args[i].arg_type != SIMD_CLONE_ARG_TYPE_UNIFORM > + && !currently_supported_simd_type (arg_type, base_type)) > { > if (TYPE_SIZE (arg_type) != TYPE_SIZE (base_type)) > warning_at (DECL_SOURCE_LOCATION (node->decl), 0, > --- gcc/testsuite/gcc.dg/gomp/pr99542.c.jj 2021-03-13 > 09:16:42.586045527 +0100 > +++ gcc/testsuite/gcc.dg/gomp/pr99542.c 2021-03-13 > 09:16:42.586045527 +0100 > @@ -0,0 +1,17 @@ > +/* PR middle-end/89246 */ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-O0 -fopenmp-simd" } */ > + > +#pragma omp declare simd > +extern int foo (__int128 x); /* { dg-warning "GCC does not currently > support mixed size types for 'simd' function" "" { target aarch64*-*-* } } */ > +/* { dg-warning "unsupported argument type '__int128' for simd" "" { target > i?86-*-* x86_64-*-* } .-1 } */ > + > +#pragma omp declare simd uniform (x) > +extern int baz (__int128 x); > + > +#pragma omp declare simd > +int > +bar (int x) > +{ > + return x + foo (0) + baz (0); > +} > --- gcc/testsuite/gcc.dg/gomp/pr59669-2.c.jj 2020-01-12 > 11:54:37.430398065 +0100 > +++ gcc/testsuite/gcc.dg/gomp/pr59669-2.c 2021-03-13 > 09:35:07.100807877 +0100 > @@ -7,4 +7,3 @@ void > bar (int *a) > { > } > -/* { dg-warning "GCC does not currently support mixed size types for 'simd' > functions" "" { target aarch64*-*-* } .-3 } */ > --- gcc/testsuite/gcc.dg/gomp/simd-clones-2.c.jj 2020-01-12 > 11:54:37.431398050 +0100 > +++ gcc/testsuite/gcc.dg/gomp/simd-clones-2.c 2021-03-13 > 09:36:21.143988256 +0100 > @@ -15,7 +15,6 @@ float setArray(float *a, float x, int k) > return a[k]; > } > > -/* { dg-warning "GCC does not currently support mixed size types for 'simd' > functions" "" { target aarch64*-*-* } .-6 } */ > /* { dg-final { scan-tree-dump "_ZGVbN4ua32vl_setArray" "optimized" > { target i?86-*-* x86_64-*-* } } } */ > /* { dg-final { scan-tree-dump "_ZGVbN4vvva32_addit" "optimized" { target > i?86-*-* x86_64-*-* } } } */ > /* { dg-final { scan-tree-dump "_ZGVbM4vl66u_addit" "optimized" { target > i?86-*-* x86_64-*-* } } } */ > --- gcc/testsuite/g++.dg/vect/simd-clone-7.cc.jj 2020-01-12 > 11:54:37.280400328 +0100 > +++ gcc/testsuite/g++.dg/vect/simd-clone-7.cc 2021-03-13 > 09:23:58.005214442 +0100 > @@ -8,5 +8,3 @@ bar (float x, float *y, int) > { > return y[0] + y[1] * x; > } > -// { dg-warning "GCC does not currently support mixed size types for 'simd' > functions" "" { target { { aarch64*-*-* } && lp64 } } .-4 } > - > --- gcc/testsuite/g++.dg/gomp/declare-simd-1.C.jj 2020-01-12 > 11:54:37.177401882 +0100 > +++ gcc/testsuite/g++.dg/gomp/declare-simd-1.C 2021-03-13 > 09:22:58.029878348 +0100 > @@ -287,7 +287,7 @@ struct D > int f37 (int a); > int e; > }; > -// { dg-warning "GCC does not currently support simdlen 16 for type 'int'" "" > { target aarch64*-*-* } .-3 } > +// { dg-warning "GCC does not currently support mixed size types for 'simd' > functions" "" { target aarch64*-*-* } .-3 } > > void > f38 (D &d) > --- gcc/testsuite/gcc.dg/declare-simd.c.jj 2020-01-12 > 11:54:37.412398337 +0100 > +++ gcc/testsuite/gcc.dg/declare-simd.c 2021-03-13 > 09:34:03.314513966 +0100 > @@ -3,6 +3,7 @@ > > #pragma omp declare simd linear (p2, p3) > extern void fn2 (float p1, float *p2, float *p3); > +/* { dg-warning "GCC does not currently support mixed size types for 'simd' > functions" "" { target aarch64*-*-* } .-1 } */ > > float *a, *b; > void fn1 (float *p1) > > Jakub
Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes: > Hi Jakub, > >> -----Original Message----- >> From: Jakub Jelinek <jakub@redhat.com> >> Sent: 13 March 2021 20:34 >> To: Richard Sandiford <Richard.Sandiford@arm.com>; Richard Earnshaw >> <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> >> Cc: gcc-patches@gcc.gnu.org >> Subject: [PATCH] aarch64: Fix up >> aarch64_simd_clone_compute_vecsize_and_simdlen [PR99542] >> >> Hi! >> >> As the patch shows, there are several bugs in >> aarch64_simd_clone_compute_vecsize_and_simdlen. >> One is that unlike for function declarations that aren't definitions >> it completely ignores argument types. Such decls don't have >> DECL_ARGUMENTS, >> but we can walk TYPE_ARG_TYPES instead, like the i386 backend does or like >> the simd cloning code in the middle end does too. >> >> Another problem is that it checks types of uniform arguments. That is >> unnecessary, uniform arguments are passed the way it normally is, it is >> a scalar argument rather than vector, so there is no reason not to support >> uniform argument of different size, or long double, structure etc. >> >> Fixed thusly, bootstrapped/regtested on aarch64-linux, ok for trunk? > > I don't have much experience in this part of GCC and I think you're best placed to make a judgement on the implementation of this hook. > It's okay from my perspective (it doesn't look like it would break any SVE logic), but if you'd like to wait for a review from Richard I won't object. Yeah, it looks good to me too, sorry for the slow response. IMO: tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE); for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i = 0; t && t != void_list_node; t = TREE_CHAIN (t), i++) { tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t); shows we have an interface problem here -- we shouldn't need something this convoluted to walk through an argument list. But that's not your problem or a problem with the patch. Thanks, Richard
On Tue, Mar 16, 2021 at 9:43 AM Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes: > > Hi Jakub, > > > >> -----Original Message----- > >> From: Jakub Jelinek <jakub@redhat.com> > >> Sent: 13 March 2021 20:34 > >> To: Richard Sandiford <Richard.Sandiford@arm.com>; Richard Earnshaw > >> <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> > >> Cc: gcc-patches@gcc.gnu.org > >> Subject: [PATCH] aarch64: Fix up > >> aarch64_simd_clone_compute_vecsize_and_simdlen [PR99542] > >> > >> Hi! > >> > >> As the patch shows, there are several bugs in > >> aarch64_simd_clone_compute_vecsize_and_simdlen. > >> One is that unlike for function declarations that aren't definitions > >> it completely ignores argument types. Such decls don't have > >> DECL_ARGUMENTS, > >> but we can walk TYPE_ARG_TYPES instead, like the i386 backend does or like > >> the simd cloning code in the middle end does too. > >> > >> Another problem is that it checks types of uniform arguments. That is > >> unnecessary, uniform arguments are passed the way it normally is, it is > >> a scalar argument rather than vector, so there is no reason not to support > >> uniform argument of different size, or long double, structure etc. > >> > >> Fixed thusly, bootstrapped/regtested on aarch64-linux, ok for trunk? > > > > I don't have much experience in this part of GCC and I think you're best placed to make a judgement on the implementation of this hook. > > It's okay from my perspective (it doesn't look like it would break any SVE logic), but if you'd like to wait for a review from Richard I won't object. > > Yeah, it looks good to me too, sorry for the slow response. > > IMO: > > tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); > bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE); > > for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i = 0; > t && t != void_list_node; t = TREE_CHAIN (t), i++) > { > tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t); > > shows we have an interface problem here -- we shouldn't need something this > convoluted to walk through an argument list. But that's not your problem > or a problem with the patch. The caller side should IMHO always look at TYPE_ARG_TYPES since that's what determines the ABI. But IIRC there were problems in the past that TYPE_ARG_TYPES might be NULL but DECL_ARGUMENTS not?! > > Thanks, > Richard
On Tue, Mar 16, 2021 at 09:46:33AM +0100, Richard Biener wrote: > > shows we have an interface problem here -- we shouldn't need something this > > convoluted to walk through an argument list. But that's not your problem > > or a problem with the patch. > > The caller side should IMHO always look at TYPE_ARG_TYPES since > that's what determines the ABI. But IIRC there were problems in the past > that TYPE_ARG_TYPES might be NULL but DECL_ARGUMENTS not?! Yes, for the K&R definitions #pragma omp declare simd int foo (a, b, c) int a, b, c; { return a + b + c; } Jakub
On Tue, Mar 16, 2021 at 10:29 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Mar 16, 2021 at 09:46:33AM +0100, Richard Biener wrote: > > > shows we have an interface problem here -- we shouldn't need something this > > > convoluted to walk through an argument list. But that's not your problem > > > or a problem with the patch. > > > > The caller side should IMHO always look at TYPE_ARG_TYPES since > > that's what determines the ABI. But IIRC there were problems in the past > > that TYPE_ARG_TYPES might be NULL but DECL_ARGUMENTS not?! > > Yes, for the K&R definitions > #pragma omp declare simd > int > foo (a, b, c) > int a, b, c; > { > return a + b + c; > } The C frontend could still populate TYPE_ARG_TYPES here, OTOH IIRC K&R definitions work like unprototyped functions on the caller side, thus having varargs argument passing semantics? > Jakub >
On Tue, Mar 16, 2021 at 10:36:35AM +0100, Richard Biener wrote: > On Tue, Mar 16, 2021 at 10:29 AM Jakub Jelinek <jakub@redhat.com> wrote: > > > > On Tue, Mar 16, 2021 at 09:46:33AM +0100, Richard Biener wrote: > > > > shows we have an interface problem here -- we shouldn't need something this > > > > convoluted to walk through an argument list. But that's not your problem > > > > or a problem with the patch. > > > > > > The caller side should IMHO always look at TYPE_ARG_TYPES since > > > that's what determines the ABI. But IIRC there were problems in the past > > > that TYPE_ARG_TYPES might be NULL but DECL_ARGUMENTS not?! > > > > Yes, for the K&R definitions > > #pragma omp declare simd > > int > > foo (a, b, c) > > int a, b, c; > > { > > return a + b + c; > > } > > The C frontend could still populate TYPE_ARG_TYPES here, OTOH IIRC > K&R definitions work like unprototyped functions on the caller side, thus > having varargs argument passing semantics? Not varargs semantics, but unprototyped function semantics, i.e. the same as for int foo (); Which means callers can pass anything, but it is not ..., i.e. callee can't use va_start/va_end/va_arg and the ... calling conventions are not in place (e.g. the passing of arguments in two places on some targets, or the extra %rax on x86_64 etc.). Jakub
On Tue, Mar 16, 2021 at 10:40 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Mar 16, 2021 at 10:36:35AM +0100, Richard Biener wrote: > > On Tue, Mar 16, 2021 at 10:29 AM Jakub Jelinek <jakub@redhat.com> wrote: > > > > > > On Tue, Mar 16, 2021 at 09:46:33AM +0100, Richard Biener wrote: > > > > > shows we have an interface problem here -- we shouldn't need something this > > > > > convoluted to walk through an argument list. But that's not your problem > > > > > or a problem with the patch. > > > > > > > > The caller side should IMHO always look at TYPE_ARG_TYPES since > > > > that's what determines the ABI. But IIRC there were problems in the past > > > > that TYPE_ARG_TYPES might be NULL but DECL_ARGUMENTS not?! > > > > > > Yes, for the K&R definitions > > > #pragma omp declare simd > > > int > > > foo (a, b, c) > > > int a, b, c; > > > { > > > return a + b + c; > > > } > > > > The C frontend could still populate TYPE_ARG_TYPES here, OTOH IIRC > > K&R definitions work like unprototyped functions on the caller side, thus > > having varargs argument passing semantics? > > Not varargs semantics, but unprototyped function semantics, i.e. the same > as for > int foo (); > Which means callers can pass anything, but it is not ..., i.e. callee can't > use va_start/va_end/va_arg and the ... calling conventions are not in place > (e.g. the passing of arguments in two places on some targets, or the > extra %rax on x86_64 etc.). Right, so for the caller side looking at DECL_ARGUMENTS is wrong as well then, the actual argument list is what matters for the used ABI. Anyway, I suppose the hook may just give up for calls to unprototyped functions? Richard. > > Jakub >
On Tue, Mar 16, 2021 at 11:16:54AM +0100, Richard Biener wrote: > > Not varargs semantics, but unprototyped function semantics, i.e. the same > > as for > > int foo (); > > Which means callers can pass anything, but it is not ..., i.e. callee can't > > use va_start/va_end/va_arg and the ... calling conventions are not in place > > (e.g. the passing of arguments in two places on some targets, or the > > extra %rax on x86_64 etc.). > > Right, so for the caller side looking at DECL_ARGUMENTS is wrong as well > then, the actual argument list is what matters for the used ABI. > > Anyway, I suppose the hook may just give up for calls to unprototyped functions? It can't. We can punt at trying to optimize them that way in the vectorizer, that is purely an optimization, but on the function definition, it is an ABI thing and perhaps some caller in some other TU could have correct prototype and call clones that don't exist. Jakub
On Tue, Mar 16, 2021 at 11:19 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Mar 16, 2021 at 11:16:54AM +0100, Richard Biener wrote: > > > Not varargs semantics, but unprototyped function semantics, i.e. the same > > > as for > > > int foo (); > > > Which means callers can pass anything, but it is not ..., i.e. callee can't > > > use va_start/va_end/va_arg and the ... calling conventions are not in place > > > (e.g. the passing of arguments in two places on some targets, or the > > > extra %rax on x86_64 etc.). > > > > Right, so for the caller side looking at DECL_ARGUMENTS is wrong as well > > then, the actual argument list is what matters for the used ABI. > > > > Anyway, I suppose the hook may just give up for calls to unprototyped functions? > > It can't. We can punt at trying to optimize them that way in the > vectorizer, that is purely an optimization, but on the function definition, > it is an ABI thing and perhaps some caller in some other TU could have correct > prototype and call clones that don't exist. I suppose we could simply reject OMP simd on K&R style definitions, OTOH they seem to be supported even in C18 ... > Jakub >
On Sat, 13 Mar 2021 at 21:34, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi! > > As the patch shows, there are several bugs in > aarch64_simd_clone_compute_vecsize_and_simdlen. > One is that unlike for function declarations that aren't definitions > it completely ignores argument types. Such decls don't have DECL_ARGUMENTS, > but we can walk TYPE_ARG_TYPES instead, like the i386 backend does or like > the simd cloning code in the middle end does too. > > Another problem is that it checks types of uniform arguments. That is > unnecessary, uniform arguments are passed the way it normally is, it is > a scalar argument rather than vector, so there is no reason not to support > uniform argument of different size, or long double, structure etc. > > Fixed thusly, bootstrapped/regtested on aarch64-linux, ok for trunk? > > 2021-03-13 Jakub Jelinek <jakub@redhat.com> > > PR target/99542 > * config/aarch64/aarch64.c > (aarch64_simd_clone_compute_vecsize_and_simdlen): If not a function > definition, walk TYPE_ARG_TYPES list if non-NULL for argument types > instead of DECL_ARGUMENTS. Ignore types for uniform arguments. > > * gcc.dg/gomp/pr99542.c: New test. > * gcc.dg/gomp/pr59669-2.c (bar): Don't expect a warning on aarch64. > * gcc.dg/gomp/simd-clones-2.c (setArray): Likewise. > * g++.dg/vect/simd-clone-7.cc (bar): Likewise. > * g++.dg/gomp/declare-simd-1.C (f37): Expect a different warning > on aarch64. > * gcc.dg/declare-simd.c (fn2): Expect a new warning on aarch64. > > --- gcc/config/aarch64/aarch64.c.jj 2021-03-12 19:01:48.672296344 +0100 > +++ gcc/config/aarch64/aarch64.c 2021-03-13 09:16:42.585045538 +0100 > @@ -23412,11 +23412,17 @@ aarch64_simd_clone_compute_vecsize_and_s > return 0; > } > > - for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t)) > + int i; > + tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); > + bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE); > + > + for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i = 0; > + t && t != void_list_node; t = TREE_CHAIN (t), i++) > { > - arg_type = TREE_TYPE (t); > + tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t); > > - if (!currently_supported_simd_type (arg_type, base_type)) > + if (clonei->args[i].arg_type != SIMD_CLONE_ARG_TYPE_UNIFORM > + && !currently_supported_simd_type (arg_type, base_type)) > { > if (TYPE_SIZE (arg_type) != TYPE_SIZE (base_type)) > warning_at (DECL_SOURCE_LOCATION (node->decl), 0, > --- gcc/testsuite/gcc.dg/gomp/pr99542.c.jj 2021-03-13 09:16:42.586045527 +0100 > +++ gcc/testsuite/gcc.dg/gomp/pr99542.c 2021-03-13 09:16:42.586045527 +0100 > @@ -0,0 +1,17 @@ > +/* PR middle-end/89246 */ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-O0 -fopenmp-simd" } */ > + > +#pragma omp declare simd > +extern int foo (__int128 x); /* { dg-warning "GCC does not currently support mixed size types for 'simd' function" "" { target aarch64*-*-* } } */ > +/* { dg-warning "unsupported argument type '__int128' for simd" "" { target i?86-*-* x86_64-*-* } .-1 } */ > + > +#pragma omp declare simd uniform (x) > +extern int baz (__int128 x); > + > +#pragma omp declare simd > +int > +bar (int x) > +{ > + return x + foo (0) + baz (0); > +} > --- gcc/testsuite/gcc.dg/gomp/pr59669-2.c.jj 2020-01-12 11:54:37.430398065 +0100 > +++ gcc/testsuite/gcc.dg/gomp/pr59669-2.c 2021-03-13 09:35:07.100807877 +0100 > @@ -7,4 +7,3 @@ void > bar (int *a) > { > } > -/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-3 } */ > --- gcc/testsuite/gcc.dg/gomp/simd-clones-2.c.jj 2020-01-12 11:54:37.431398050 +0100 > +++ gcc/testsuite/gcc.dg/gomp/simd-clones-2.c 2021-03-13 09:36:21.143988256 +0100 > @@ -15,7 +15,6 @@ float setArray(float *a, float x, int k) > return a[k]; > } > > -/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-6 } */ > /* { dg-final { scan-tree-dump "_ZGVbN4ua32vl_setArray" "optimized" { target i?86-*-* x86_64-*-* } } } */ > /* { dg-final { scan-tree-dump "_ZGVbN4vvva32_addit" "optimized" { target i?86-*-* x86_64-*-* } } } */ > /* { dg-final { scan-tree-dump "_ZGVbM4vl66u_addit" "optimized" { target i?86-*-* x86_64-*-* } } } */ > --- gcc/testsuite/g++.dg/vect/simd-clone-7.cc.jj 2020-01-12 11:54:37.280400328 +0100 > +++ gcc/testsuite/g++.dg/vect/simd-clone-7.cc 2021-03-13 09:23:58.005214442 +0100 > @@ -8,5 +8,3 @@ bar (float x, float *y, int) > { > return y[0] + y[1] * x; > } > -// { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target { { aarch64*-*-* } && lp64 } } .-4 } > - > --- gcc/testsuite/g++.dg/gomp/declare-simd-1.C.jj 2020-01-12 11:54:37.177401882 +0100 > +++ gcc/testsuite/g++.dg/gomp/declare-simd-1.C 2021-03-13 09:22:58.029878348 +0100 > @@ -287,7 +287,7 @@ struct D > int f37 (int a); > int e; > }; > -// { dg-warning "GCC does not currently support simdlen 16 for type 'int'" "" { target aarch64*-*-* } .-3 } > +// { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-3 } > > void > f38 (D &d) > --- gcc/testsuite/gcc.dg/declare-simd.c.jj 2020-01-12 11:54:37.412398337 +0100 > +++ gcc/testsuite/gcc.dg/declare-simd.c 2021-03-13 09:34:03.314513966 +0100 > @@ -3,6 +3,7 @@ > > #pragma omp declare simd linear (p2, p3) > extern void fn2 (float p1, float *p2, float *p3); > +/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-1 } */ > > float *a, *b; > void fn1 (float *p1) > Unfortunately, the last new warning does not happen when running the tests with -mabi=ilp32. Is that small patch OK? diff --git a/gcc/testsuite/gcc.dg/declare-simd.c b/gcc/testsuite/gcc.dg/declare-simd.c index 52796f6..894fb64 100644 --- a/gcc/testsuite/gcc.dg/declare-simd.c +++ b/gcc/testsuite/gcc.dg/declare-simd.c @@ -3,7 +3,7 @@ #pragma omp declare simd linear (p2, p3) extern void fn2 (float p1, float *p2, float *p3); -/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-1 } */ +/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target { { aarch64*-*-* } && { ! ilp32 } } } .-1 } */ float *a, *b; void fn1 (float *p1) Thanks Christophe > Jakub >
On Tue, Mar 16, 2021 at 06:34:34PM +0100, Christophe Lyon wrote: > On Sat, 13 Mar 2021 at 21:34, Jakub Jelinek via Gcc-patches > Unfortunately, the last new warning does not happen when running the > tests with -mabi=ilp32. > Is that small patch OK? With a proper ChangeLog entry, sure. Though perhaps it would be easier to do && lp64. Your choice. > diff --git a/gcc/testsuite/gcc.dg/declare-simd.c > b/gcc/testsuite/gcc.dg/declare-simd.c > index 52796f6..894fb64 100644 > --- a/gcc/testsuite/gcc.dg/declare-simd.c > +++ b/gcc/testsuite/gcc.dg/declare-simd.c > @@ -3,7 +3,7 @@ > > #pragma omp declare simd linear (p2, p3) > extern void fn2 (float p1, float *p2, float *p3); > -/* { dg-warning "GCC does not currently support mixed size types for > 'simd' functions" "" { target aarch64*-*-* } .-1 } */ > +/* { dg-warning "GCC does not currently support mixed size types for > 'simd' functions" "" { target { { aarch64*-*-* } && { ! ilp32 } } } > .-1 } */ > > float *a, *b; > void fn1 (float *p1) Jakub
On Tue, 16 Mar 2021 at 18:38, Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Mar 16, 2021 at 06:34:34PM +0100, Christophe Lyon wrote: > > On Sat, 13 Mar 2021 at 21:34, Jakub Jelinek via Gcc-patches > > Unfortunately, the last new warning does not happen when running the > > tests with -mabi=ilp32. > > Is that small patch OK? > > With a proper ChangeLog entry, sure. > Though perhaps it would be easier to do && lp64. Your choice. Indeed, I'll do that. Thanks > > > diff --git a/gcc/testsuite/gcc.dg/declare-simd.c > > b/gcc/testsuite/gcc.dg/declare-simd.c > > index 52796f6..894fb64 100644 > > --- a/gcc/testsuite/gcc.dg/declare-simd.c > > +++ b/gcc/testsuite/gcc.dg/declare-simd.c > > @@ -3,7 +3,7 @@ > > > > #pragma omp declare simd linear (p2, p3) > > extern void fn2 (float p1, float *p2, float *p3); > > -/* { dg-warning "GCC does not currently support mixed size types for > > 'simd' functions" "" { target aarch64*-*-* } .-1 } */ > > +/* { dg-warning "GCC does not currently support mixed size types for > > 'simd' functions" "" { target { { aarch64*-*-* } && { ! ilp32 } } } > > .-1 } */ > > > > float *a, *b; > > void fn1 (float *p1) > > Jakub >
--- gcc/config/aarch64/aarch64.c.jj 2021-03-12 19:01:48.672296344 +0100 +++ gcc/config/aarch64/aarch64.c 2021-03-13 09:16:42.585045538 +0100 @@ -23412,11 +23412,17 @@ aarch64_simd_clone_compute_vecsize_and_s return 0; } - for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t)) + int i; + tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); + bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE); + + for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i = 0; + t && t != void_list_node; t = TREE_CHAIN (t), i++) { - arg_type = TREE_TYPE (t); + tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t); - if (!currently_supported_simd_type (arg_type, base_type)) + if (clonei->args[i].arg_type != SIMD_CLONE_ARG_TYPE_UNIFORM + && !currently_supported_simd_type (arg_type, base_type)) { if (TYPE_SIZE (arg_type) != TYPE_SIZE (base_type)) warning_at (DECL_SOURCE_LOCATION (node->decl), 0, --- gcc/testsuite/gcc.dg/gomp/pr99542.c.jj 2021-03-13 09:16:42.586045527 +0100 +++ gcc/testsuite/gcc.dg/gomp/pr99542.c 2021-03-13 09:16:42.586045527 +0100 @@ -0,0 +1,17 @@ +/* PR middle-end/89246 */ +/* { dg-do compile { target int128 } } */ +/* { dg-options "-O0 -fopenmp-simd" } */ + +#pragma omp declare simd +extern int foo (__int128 x); /* { dg-warning "GCC does not currently support mixed size types for 'simd' function" "" { target aarch64*-*-* } } */ +/* { dg-warning "unsupported argument type '__int128' for simd" "" { target i?86-*-* x86_64-*-* } .-1 } */ + +#pragma omp declare simd uniform (x) +extern int baz (__int128 x); + +#pragma omp declare simd +int +bar (int x) +{ + return x + foo (0) + baz (0); +} --- gcc/testsuite/gcc.dg/gomp/pr59669-2.c.jj 2020-01-12 11:54:37.430398065 +0100 +++ gcc/testsuite/gcc.dg/gomp/pr59669-2.c 2021-03-13 09:35:07.100807877 +0100 @@ -7,4 +7,3 @@ void bar (int *a) { } -/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-3 } */ --- gcc/testsuite/gcc.dg/gomp/simd-clones-2.c.jj 2020-01-12 11:54:37.431398050 +0100 +++ gcc/testsuite/gcc.dg/gomp/simd-clones-2.c 2021-03-13 09:36:21.143988256 +0100 @@ -15,7 +15,6 @@ float setArray(float *a, float x, int k) return a[k]; } -/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-6 } */ /* { dg-final { scan-tree-dump "_ZGVbN4ua32vl_setArray" "optimized" { target i?86-*-* x86_64-*-* } } } */ /* { dg-final { scan-tree-dump "_ZGVbN4vvva32_addit" "optimized" { target i?86-*-* x86_64-*-* } } } */ /* { dg-final { scan-tree-dump "_ZGVbM4vl66u_addit" "optimized" { target i?86-*-* x86_64-*-* } } } */ --- gcc/testsuite/g++.dg/vect/simd-clone-7.cc.jj 2020-01-12 11:54:37.280400328 +0100 +++ gcc/testsuite/g++.dg/vect/simd-clone-7.cc 2021-03-13 09:23:58.005214442 +0100 @@ -8,5 +8,3 @@ bar (float x, float *y, int) { return y[0] + y[1] * x; } -// { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target { { aarch64*-*-* } && lp64 } } .-4 } - --- gcc/testsuite/g++.dg/gomp/declare-simd-1.C.jj 2020-01-12 11:54:37.177401882 +0100 +++ gcc/testsuite/g++.dg/gomp/declare-simd-1.C 2021-03-13 09:22:58.029878348 +0100 @@ -287,7 +287,7 @@ struct D int f37 (int a); int e; }; -// { dg-warning "GCC does not currently support simdlen 16 for type 'int'" "" { target aarch64*-*-* } .-3 } +// { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-3 } void f38 (D &d) --- gcc/testsuite/gcc.dg/declare-simd.c.jj 2020-01-12 11:54:37.412398337 +0100 +++ gcc/testsuite/gcc.dg/declare-simd.c 2021-03-13 09:34:03.314513966 +0100 @@ -3,6 +3,7 @@ #pragma omp declare simd linear (p2, p3) extern void fn2 (float p1, float *p2, float *p3); +/* { dg-warning "GCC does not currently support mixed size types for 'simd' functions" "" { target aarch64*-*-* } .-1 } */ float *a, *b; void fn1 (float *p1)