| Message ID | 20160603173343.GM7387@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 3 Jun 2016, Jakub Jelinek wrote: > On Tue, Jan 12, 2016 at 05:21:37PM +0300, Ilya Enkovich wrote: > > > --- gcc/tree-vect-slp.c.jj 2016-01-08 21:45:57.000000000 +0100 > > > +++ gcc/tree-vect-slp.c 2016-01-11 12:07:19.633366712 +0100 > > > @@ -2999,12 +2999,9 @@ vect_get_constant_vectors (tree op, slp_ > > > gimple *init_stmt; > > > if (VECTOR_BOOLEAN_TYPE_P (vector_type)) > > > { > > > - gcc_assert (fold_convertible_p (TREE_TYPE (vector_type), > > > - op)); > > > + gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (op))); > > > init_stmt = gimple_build_assign (new_temp, NOP_EXPR, op); > > > > In vect_init_vector we had to introduce COND_EXPR to choose between 0 and -1 for > > boolean vectors. Shouldn't we do similar in SLP? > > Apparently the answer to this is YES. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.2? > > 2016-06-03 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/71259 > * tree-vect-slp.c (vect_get_constant_vectors): For > VECTOR_BOOLEAN_TYPE_P, return all ones constant instead of > one for constant op, and use COND_EXPR for non-constant. > > * gcc.dg/vect/pr71259.c: New test. > > --- gcc/tree-vect-slp.c.jj 2016-05-24 10:56:02.000000000 +0200 > +++ gcc/tree-vect-slp.c 2016-06-03 17:01:12.740955935 +0200 > @@ -3056,7 +3056,7 @@ vect_get_constant_vectors (tree op, slp_ > if (integer_zerop (op)) > op = build_int_cst (TREE_TYPE (vector_type), 0); > else if (integer_onep (op)) > - op = build_int_cst (TREE_TYPE (vector_type), 1); > + op = build_all_ones_cst (TREE_TYPE (vector_type)); > else > gcc_unreachable (); > } > @@ -3071,8 +3071,14 @@ vect_get_constant_vectors (tree op, slp_ > gimple *init_stmt; > if (VECTOR_BOOLEAN_TYPE_P (vector_type)) > { > + tree true_val > + = build_all_ones_cst (TREE_TYPE (vector_type)); > + tree false_val > + = build_zero_cst (TREE_TYPE (vector_type)); > gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (op))); > - init_stmt = gimple_build_assign (new_temp, NOP_EXPR, op); > + init_stmt = gimple_build_assign (new_temp, COND_EXPR, > + op, true_val, > + false_val); So this ends up generating { a ? -1 : 0, b ? -1 : 0, ... }. That might be less optimal than doing { a, b, ... } ? { -1, -1 ... } : { 0, 0, .. } though I'm not sure we can easily construct a "proper" vector boolean from _Bool values either. Thus the patch is ok. Thanks, Richard. > } > else > { > --- 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" } */ > +/* { 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; > +} > > > Jakub > >
On Mon, Jun 06, 2016 at 10:05:57AM +0200, Richard Biener wrote: > So this ends up generating { a ? -1 : 0, b ? -1 : 0, ... }. That Yes, that is already what we do now for loop vectorization. > might be less optimal than doing { a, b, ... } ? { -1, -1 ... } : { 0, 0, > .. } Well, it would need to be { a, b, ... } != { 0, 0, ... } ? { -1, -1, ... } : { 0, 0, ... } then, doesn't VEC_COND_EXPR assume the condition is in canonical VECTOR_BOOLEAN_TYPE_P form? Anyway, if something like the above would be faster, perhaps generic vector lowering or some similar pass could detect that case post-vectorization and optimize? Jakub
On Mon, 6 Jun 2016, Jakub Jelinek wrote: > On Mon, Jun 06, 2016 at 10:05:57AM +0200, Richard Biener wrote: > > So this ends up generating { a ? -1 : 0, b ? -1 : 0, ... }. That > > Yes, that is already what we do now for loop vectorization. > > > might be less optimal than doing { a, b, ... } ? { -1, -1 ... } : { 0, 0, > > .. } > > Well, it would need to be > { a, b, ... } != { 0, 0, ... } ? { -1, -1, ... } : { 0, 0, ... } > then, doesn't VEC_COND_EXPR assume the condition is in canonical > VECTOR_BOOLEAN_TYPE_P form? > > Anyway, if something like the above would be faster, perhaps generic vector > lowering or some similar pass could detect that case post-vectorization and > optimize? Yeah. OTOH an "ideal" vectorizer would already consider the smaller prologue cost for its cost modeling. Richard.
Hi Jakub, On 3 June 2016 at 19:33, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Jan 12, 2016 at 05:21:37PM +0300, Ilya Enkovich wrote: >> > --- gcc/tree-vect-slp.c.jj 2016-01-08 21:45:57.000000000 +0100 >> > +++ gcc/tree-vect-slp.c 2016-01-11 12:07:19.633366712 +0100 >> > @@ -2999,12 +2999,9 @@ vect_get_constant_vectors (tree op, slp_ >> > gimple *init_stmt; >> > if (VECTOR_BOOLEAN_TYPE_P (vector_type)) >> > { >> > - gcc_assert (fold_convertible_p (TREE_TYPE (vector_type), >> > - op)); >> > + gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (op))); >> > init_stmt = gimple_build_assign (new_temp, NOP_EXPR, op); >> >> In vect_init_vector we had to introduce COND_EXPR to choose between 0 and -1 for >> boolean vectors. Shouldn't we do similar in SLP? > > Apparently the answer to this is YES. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.2? > > 2016-06-03 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/71259 > * tree-vect-slp.c (vect_get_constant_vectors): For > VECTOR_BOOLEAN_TYPE_P, return all ones constant instead of > one for constant op, and use COND_EXPR for non-constant. > > * gcc.dg/vect/pr71259.c: New test. > > --- gcc/tree-vect-slp.c.jj 2016-05-24 10:56:02.000000000 +0200 > +++ gcc/tree-vect-slp.c 2016-06-03 17:01:12.740955935 +0200 > @@ -3056,7 +3056,7 @@ vect_get_constant_vectors (tree op, slp_ > if (integer_zerop (op)) > op = build_int_cst (TREE_TYPE (vector_type), 0); > else if (integer_onep (op)) > - op = build_int_cst (TREE_TYPE (vector_type), 1); > + op = build_all_ones_cst (TREE_TYPE (vector_type)); > else > gcc_unreachable (); > } > @@ -3071,8 +3071,14 @@ vect_get_constant_vectors (tree op, slp_ > gimple *init_stmt; > if (VECTOR_BOOLEAN_TYPE_P (vector_type)) > { > + tree true_val > + = build_all_ones_cst (TREE_TYPE (vector_type)); > + tree false_val > + = build_zero_cst (TREE_TYPE (vector_type)); > gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (op))); > - init_stmt = gimple_build_assign (new_temp, NOP_EXPR, op); > + init_stmt = gimple_build_assign (new_temp, COND_EXPR, > + op, true_val, > + false_val); > } > else > { > --- 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" } */ > +/* { 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? Christophe. > > Jakub
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. > > +/* { 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
On Tue, Jun 7, 2016 at 10:28 AM, 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. all tests in gcc.dg/vect have some form of dg-require-effective-target - so I think this test should just have dg-require-effective-target "vect_int" . Ramana > >> > +/* { 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
On Tue, Jun 07, 2016 at 10:36:25AM +0100, Ramana Radhakrishnan wrote: > On Tue, Jun 7, 2016 at 10:28 AM, 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. > > > all tests in gcc.dg/vect have some form of dg-require-effective-target No, at least 170+ tests don't. > - so I think this test should just have dg-require-effective-target > "vect_int" . No, why? This test doesn't test whether the function has been vectorized. It only tests whether it works. And the check_vect () is supposed to exit early if some extra flags were passed by vect.exp (like e.g. on i?86-linux -msse2) and the HW doesn't support those. Jakub
On 7 June 2016 at 11:42, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Jun 07, 2016 at 10:36:25AM +0100, Ramana Radhakrishnan wrote: >> On Tue, Jun 7, 2016 at 10:28 AM, 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. >> >> >> all tests in gcc.dg/vect have some form of dg-require-effective-target > > No, at least 170+ tests don't. > >> - so I think this test should just have dg-require-effective-target >> "vect_int" . > > No, why? This test doesn't test whether the function has been vectorized. > It only tests whether it works. > And the check_vect () is supposed to exit early if some extra flags were > passed by vect.exp (like e.g. on i?86-linux -msse2) and the HW doesn't > support those. > But this makes the tests fails, rather than be unsupported, right? > Jakub
On Tue, Jun 07, 2016 at 02:43:37PM +0200, Christophe Lyon wrote: > > No, why? This test doesn't test whether the function has been vectorized. > > It only tests whether it works. > > And the check_vect () is supposed to exit early if some extra flags were > > passed by vect.exp (like e.g. on i?86-linux -msse2) and the HW doesn't > > support those. > > > But this makes the tests fails, rather than be unsupported, right? check_vect is supposed to exit (0) if the HW doesn't support vectorization, so the test would PASS. Jakub
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) 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
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 > >
On Wed, Jun 08, 2016 at 12:26:17PM +0200, Richard Biener wrote: > > 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. All tests in {gcc,g++}.dg/vect/, right? I agree with that. Jakub
On Wed, 8 Jun 2016, Jakub Jelinek wrote: > On Wed, Jun 08, 2016 at 12:26:17PM +0200, Richard Biener wrote: > > > 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. > > All tests in {gcc,g++}.dg/vect/, right? I agree with that. Yes. [and most of the vect.exp fancy-filename stuff should be replaced by adding dg-additional-options] Richard.
--- gcc/tree-vect-slp.c.jj 2016-05-24 10:56:02.000000000 +0200 +++ gcc/tree-vect-slp.c 2016-06-03 17:01:12.740955935 +0200 @@ -3056,7 +3056,7 @@ vect_get_constant_vectors (tree op, slp_ if (integer_zerop (op)) op = build_int_cst (TREE_TYPE (vector_type), 0); else if (integer_onep (op)) - op = build_int_cst (TREE_TYPE (vector_type), 1); + op = build_all_ones_cst (TREE_TYPE (vector_type)); else gcc_unreachable (); } @@ -3071,8 +3071,14 @@ vect_get_constant_vectors (tree op, slp_ gimple *init_stmt; if (VECTOR_BOOLEAN_TYPE_P (vector_type)) { + tree true_val + = build_all_ones_cst (TREE_TYPE (vector_type)); + tree false_val + = build_zero_cst (TREE_TYPE (vector_type)); gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (op))); - init_stmt = gimple_build_assign (new_temp, NOP_EXPR, op); + init_stmt = gimple_build_assign (new_temp, COND_EXPR, + op, true_val, + false_val); } else { --- 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" } */ +/* { 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; +}