| Message ID | mptftexvak1.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | optabs: Don't use scalar conversions for vectors [PR93843] | expand |
On Wed, Feb 26, 2020 at 12:43 PM Richard Sandiford <richard.sandiford@arm.com> wrote: > > In this PR we had a conversion between two integer vectors that > both had scalar integer modes. We then tried to implement the > conversion using the scalar optab for those modes, instead of > doing the conversion elementwise. > > I wondered about letting through scalar modes for single-element > vectors, but I don't have any evidence that that's useful/necessary, > so it seemed better to keep things simple. > > Tested on aarch64-linux-gnu, armeb-eabi and x86_64-linux-gnu. > OK to install? OK. Thanks, Richard. > Richard > > > 2020-02-26 Richard Sandiford <richard.sandiford@arm.com> > > gcc/ > PR middle-end/93843 > * optabs-tree.c (supportable_convert_operation): Reject types with > scalar modes. > > gcc/testsuite/ > PR middle-end/93843 > * gcc.dg/vect/pr93843-1.c: New test. > * gcc.dg/vect/pr93843-2.c: Likewise. > --- > gcc/optabs-tree.c | 5 +++++ > gcc/testsuite/gcc.dg/vect/pr93843-1.c | 21 +++++++++++++++++++++ > gcc/testsuite/gcc.dg/vect/pr93843-2.c | 11 +++++++++++ > 3 files changed, 37 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/vect/pr93843-1.c > create mode 100644 gcc/testsuite/gcc.dg/vect/pr93843-2.c > > diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c > index 3d829c27826..badd30bfda8 100644 > --- a/gcc/optabs-tree.c > +++ b/gcc/optabs-tree.c > @@ -284,9 +284,14 @@ supportable_convert_operation (enum tree_code code, > machine_mode m1,m2; > bool truncp; > > + gcc_assert (VECTOR_TYPE_P (vectype_out) && VECTOR_TYPE_P (vectype_in)); > + > m1 = TYPE_MODE (vectype_out); > m2 = TYPE_MODE (vectype_in); > > + if (!VECTOR_MODE_P (m1) || !VECTOR_MODE_P (m2)) > + return false; > + > /* First check if we can done conversion directly. */ > if ((code == FIX_TRUNC_EXPR > && can_fix_p (m1,m2,TYPE_UNSIGNED (vectype_out), &truncp) > diff --git a/gcc/testsuite/gcc.dg/vect/pr93843-1.c b/gcc/testsuite/gcc.dg/vect/pr93843-1.c > new file mode 100644 > index 00000000000..23a79ca4c96 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/pr93843-1.c > @@ -0,0 +1,21 @@ > +char a; > +struct S { short b, c; } d; > + > +__attribute__((noipa)) void > +foo (int x) > +{ > + if (x != 4) > + __builtin_abort (); > +} > + > +int > +main () > +{ > + short *g = &d.c, *h = &d.b; > + char e = 4 - a; > + int f; > + *h = *g = e; > + for (f = 0; f < 2; f++) > + foo (d.c); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/vect/pr93843-2.c b/gcc/testsuite/gcc.dg/vect/pr93843-2.c > new file mode 100644 > index 00000000000..5fae3e5be17 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/pr93843-2.c > @@ -0,0 +1,11 @@ > +char in[2] = {2, 2}; > +short out[2] = {}; > + > +int > +main() > +{ > + for (int i = 0; i < 2; ++i) > + out[i] = in[i]; > + asm("":::"memory"); > + if (out[0] != 2) __builtin_abort(); > +}
On Wed, Feb 26, 2020 at 11:43:10AM +0000, Richard Sandiford wrote: > In this PR we had a conversion between two integer vectors that > both had scalar integer modes. We then tried to implement the > conversion using the scalar optab for those modes, instead of > doing the conversion elementwise. > > I wondered about letting through scalar modes for single-element > vectors, but I don't have any evidence that that's useful/necessary, > so it seemed better to keep things simple. > > Tested on aarch64-linux-gnu, armeb-eabi and x86_64-linux-gnu. Won't this prevent even say __v4qi to __v4uqi and similar conversions with scalar modes for those where we don't need any kind of extensions, just reinterpret the bits? Jakub
Jakub Jelinek <jakub@redhat.com> writes: > On Wed, Feb 26, 2020 at 11:43:10AM +0000, Richard Sandiford wrote: >> In this PR we had a conversion between two integer vectors that >> both had scalar integer modes. We then tried to implement the >> conversion using the scalar optab for those modes, instead of >> doing the conversion elementwise. >> >> I wondered about letting through scalar modes for single-element >> vectors, but I don't have any evidence that that's useful/necessary, >> so it seemed better to keep things simple. >> >> Tested on aarch64-linux-gnu, armeb-eabi and x86_64-linux-gnu. > > Won't this prevent even say __v4qi to __v4uqi and similar conversions > with scalar modes for those where we don't need any kind of extensions, > just reinterpret the bits? Those kinds of conversions aren't accepted by the function anyway, even for vector modes, since there's no associated optab that they could use. The vectoriser handles them using VCEs instead, via vectorizable_assignment rather than vectorizable_conversion. I guess for GCC 11 we could allow NOP_EXPR to be used for reinterpreting signedness if that seems like the right thing to do. Thanks, Richard
diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c index 3d829c27826..badd30bfda8 100644 --- a/gcc/optabs-tree.c +++ b/gcc/optabs-tree.c @@ -284,9 +284,14 @@ supportable_convert_operation (enum tree_code code, machine_mode m1,m2; bool truncp; + gcc_assert (VECTOR_TYPE_P (vectype_out) && VECTOR_TYPE_P (vectype_in)); + m1 = TYPE_MODE (vectype_out); m2 = TYPE_MODE (vectype_in); + if (!VECTOR_MODE_P (m1) || !VECTOR_MODE_P (m2)) + return false; + /* First check if we can done conversion directly. */ if ((code == FIX_TRUNC_EXPR && can_fix_p (m1,m2,TYPE_UNSIGNED (vectype_out), &truncp) diff --git a/gcc/testsuite/gcc.dg/vect/pr93843-1.c b/gcc/testsuite/gcc.dg/vect/pr93843-1.c new file mode 100644 index 00000000000..23a79ca4c96 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/pr93843-1.c @@ -0,0 +1,21 @@ +char a; +struct S { short b, c; } d; + +__attribute__((noipa)) void +foo (int x) +{ + if (x != 4) + __builtin_abort (); +} + +int +main () +{ + short *g = &d.c, *h = &d.b; + char e = 4 - a; + int f; + *h = *g = e; + for (f = 0; f < 2; f++) + foo (d.c); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/vect/pr93843-2.c b/gcc/testsuite/gcc.dg/vect/pr93843-2.c new file mode 100644 index 00000000000..5fae3e5be17 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/pr93843-2.c @@ -0,0 +1,11 @@ +char in[2] = {2, 2}; +short out[2] = {}; + +int +main() +{ + for (int i = 0; i < 2; ++i) + out[i] = in[i]; + asm("":::"memory"); + if (out[0] != 2) __builtin_abort(); +}