Re: [PATCH] Fix SLP wrong-code with VECTOR_BOOLEAN_TYPE_P (PR tree-optimization/71259)

[Richard Biener <rguenther@suse.de>]

On Wed, 8 Jun 2016, Christophe Lyon wrote:

> On 7 June 2016 at 11:28, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Tue, Jun 07, 2016 at 11:23:01AM +0200, Christophe Lyon wrote:
> >> > --- gcc/testsuite/gcc.dg/vect/pr71259.c.jj      2016-06-03 17:05:37.693475438 +0200
> >> > +++ gcc/testsuite/gcc.dg/vect/pr71259.c 2016-06-03 17:05:32.418544731 +0200
> >> > @@ -0,0 +1,28 @@
> >> > +/* PR tree-optimization/71259 */
> >> > +/* { dg-do run } */
> >> > +/* { dg-options "-O3" } */
> >
> > Would changing this from dg-options to dg-additional-options help for the
> > ARM issues?
> > check_vect () is the standard way for testing for HW vectorization support
> > and hundreds of tests use it.
> >
> 
> This does fix the problem for pr71259.
> I've also tried to replace all the dg-options by dg-additional-options
> in vect/*.c, and this improves:
> gcc.dg/vect/vect-shift-2-big-array.c
> gcc.dg/vect/vect-shift-2.c
> 
> It has no effect on arm/aarch64 on these tests (which already pass or
> are unsupported):
> no-tree-pre-pr45241.c
> pr18308.c
> pr24049.c
> pr33373.c
> pr36228.c
> pr42395.c
> pr42604.c
> pr46663.c
> (unsupported) pr48765.c
> pr49093.c
> pr49352.c
> pr52298.c
> pr52870.c
> pr53185.c
> pr53773.c
> pr56695.c
> (unsupported) pr62171.c
> pr63530.c
> pr68339.c
> (unsupported) vect-82_64.c
> (unsupported) vect-83_64.c
> vect-debug-pr41926.c
> vect-fold-1.c
> vect-singleton_1.c
> 
> So: should I change dg-options into dg-additional-options for all the
> tests for consistency, or only on the 3 ones where it makes them pass?
> (pr71259.c, vect-shift-2-big-array.c, vect-shift-2.c)

I think all tests should use dg-additional-options.

Richard.

> Thanks
> 
> Christophe.
> 
> >> > +/* { dg-additional-options "-mavx" { target avx_runtime } } */
> >> > +
> >> > +#include "tree-vect.h"
> >> > +
> >> > +long a, b[1][44][2];
> >> > +long long c[44][17][2];
> >> > +
> >> > +int
> >> > +main ()
> >> > +{
> >> > +  int i, j, k;
> >> > +  check_vect ();
> >> > +  asm volatile ("" : : : "memory");
> >> > +  for (i = 0; i < 44; i++)
> >> > +    for (j = 0; j < 17; j++)
> >> > +      for (k = 0; k < 2; k++)
> >> > +       c[i][j][k] = (30995740 >= *(k + *(j + *b)) != (a != 8)) - 5105075050047261684;
> >> > +  asm volatile ("" : : : "memory");
> >> > +  for (i = 0; i < 44; i++)
> >> > +    for (j = 0; j < 17; j++)
> >> > +      for (k = 0; k < 2; k++)
> >> > +       if (c[i][j][k] != -5105075050047261684)
> >> > +         __builtin_abort ();
> >> > +  return 0;
> >> > +}
> >> >
> >>
> >> This new test fails on ARM targets where the default FPU is not Neon like.
> >> The error message I'm seeing is:
> >> In file included from
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/vect/pr71259.c:6:0:
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/vect/tree-vect.h:
> >> In function 'check_vect':
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/vect/tree-vect.h:65:5:
> >> error: inconsistent operand constraints in an 'asm'
> >>
> >> Well, the same error message actually appears with other tests, I did
> >> notice this one because
> >> it is a new one.
> >>
> >> The arm code is:
> >>     /* On some processors without NEON support, this instruction may
> >>        be a no-op, on others it may trap, so check that it executes
> >>        correctly.  */
> >>     long long a = 0, b = 1;
> >>     asm ("vorr %P0, %P1, %P2"
> >>          : "=w" (a)
> >>          : "0" (a), "w" (b));
> >>
> >> ... which has been here since 2007 :(
> >>
> >> IIUC, its purpose is to check Neon availability, but this makes the
> >> tests fail instead of
> >> being unsupported.
> >>
> >> Why not use an effective-target check instead?
> >
> >         Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)