On 19/05/2020 17:59, Richard Sandiford wrote:
> Alex Coplan <alex.cop...@arm.com> writes:
> > Hello,
> >
> > This patch fixes PR94591. The problem was the function 
> > aarch64_evpc_rev_local()
> > matching vector permutations that were not reversals. In particular, prior 
> > to
> > this patch, this function matched the identity permutation which led to
> > generating bogus REV64 insns which were rejected by the assembler.
> >
> > Testing:
> >  - New regression test which passes after applying the patch.
> >  - New test passes on an x64 -> aarch64-none-elf cross.
> >  - Bootstrap and regtest on aarch64-linux-gnu.
> >
> > OK to install?
> >
> > Thanks,
> > Alex
> >
> > ---
> >
> > gcc/ChangeLog:
> >
> > 2020-05-19  Alex Coplan  <alex.cop...@arm.com>
> >
> >     PR target/94591
> >     * config/aarch64/aarch64.c (aarch64_evpc_rev_local): Don't match
> >     identity permutation.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2020-05-19  Alex Coplan  <alex.cop...@arm.com>
> >
> >     PR target/94591
> >     * gcc.c-torture/execute/pr94591.c: New test.
> 
> OK, thanks.
> 
> Richard

I've just tested this patch on gcc-{8,9,10} release branches:
bootstraps+regtests on aarch64-linux-gnu came back clean.

Since this was a regression introduced in GCC 8, is it OK to backport
the fix to those release branches now?

Thanks,
Alex

> 
> > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > index 70aa2f752b5..79c016f4dc3 100644
> > --- a/gcc/config/aarch64/aarch64.c
> > +++ b/gcc/config/aarch64/aarch64.c
> > @@ -20191,7 +20191,8 @@ aarch64_evpc_rev_local (struct expand_vec_perm_d *d)
> >  
> >    if (d->vec_flags == VEC_SVE_PRED
> >        || !d->one_vector_p
> > -      || !d->perm[0].is_constant (&diff))
> > +      || !d->perm[0].is_constant (&diff)
> > +      || !diff)
> >      return false;
> >  
> >    size = (diff + 1) * GET_MODE_UNIT_SIZE (d->vmode);
> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94591.c 
> > b/gcc/testsuite/gcc.c-torture/execute/pr94591.c
> > new file mode 100644
> > index 00000000000..42271ad8bce
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94591.c
> > @@ -0,0 +1,32 @@
> > +typedef unsigned __attribute__((__vector_size__(8))) V2SI_u;
> > +typedef int __attribute__((__vector_size__(8))) V2SI_d;
> > +
> > +typedef unsigned long __attribute__((__vector_size__(16))) V2DI_u;
> > +typedef long __attribute__((__vector_size__(16))) V2DI_d;
> > +
> > +void id_V2SI(V2SI_d *v)
> > +{
> > +  *v = __builtin_shuffle(*v, (V2SI_d)(V2SI_u) { 0, 1 });
> > +}
> > +
> > +void id_V2DI(V2DI_d *v)
> > +{
> > +  *v = __builtin_shuffle(*v, (V2DI_d)(V2DI_u) { 0, 1 });
> > +}
> > +
> > +extern void abort(void);
> > +
> > +int main(void)
> > +{
> > +  V2SI_d si = { 35, 42 };
> > +  id_V2SI(&si);
> > +
> > +  if (si[0] != 35 || si[1] != 42)
> > +    abort();
> > +
> > +  V2DI_d di = { 63, 38 };
> > +  id_V2DI(&di);
> > +
> > +  if (di[0] != 63 || di[1] != 38)
> > +    abort();
> > +}

Reply via email to