diff mbox series

PR83004: Accidental change to pr81136.c for VECTOR_BITS==128

Message ID 8760a21wzd.fsf@linaro.org
State New
Headers show
Series PR83004: Accidental change to pr81136.c for VECTOR_BITS==128 | expand

Commit Message

Richard Sandiford Nov. 22, 2017, 9:30 a.m. UTC 
r254589 was supposed to leave tests unchanged for the default setting
of VECTOR_BITS, but I must have got my sums wrong on pr81136.c.
Sorry for the breakage.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,
OK to install?

Richard


2017-11-22  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/testsuite/
	PR testsuite/83004
	* gcc.dg/vect/pr81136.c: Restore previous alignment of 32
	in the default case.

Comments

Richard Biener Nov. 22, 2017, 1:51 p.m. UTC | #1 
On Wed, Nov 22, 2017 at 10:30 AM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> r254589 was supposed to leave tests unchanged for the default setting

> of VECTOR_BITS, but I must have got my sums wrong on pr81136.c.

> Sorry for the breakage.

>

> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,

> OK to install?


Ok.

Richard.

> Richard

>

>

> 2017-11-22  Richard Sandiford  <richard.sandiford@linaro.org>

>

> gcc/testsuite/

>         PR testsuite/83004

>         * gcc.dg/vect/pr81136.c: Restore previous alignment of 32

>         in the default case.

>

> Index: gcc/testsuite/gcc.dg/vect/pr81136.c

> ===================================================================

> --- gcc/testsuite/gcc.dg/vect/pr81136.c 2017-11-22 09:21:12.538474075 +0000

> +++ gcc/testsuite/gcc.dg/vect/pr81136.c 2017-11-22 09:28:04.836628929 +0000

> @@ -2,7 +2,13 @@

>

>  #include "tree-vect.h"

>

> -struct __attribute__((aligned (VECTOR_BITS / 8)))

> +#if VECTOR_BITS > 256

> +#define ALIGNMENT (VECTOR_BITS / 8)

> +#else

> +#define ALIGNMENT 32

> +#endif

> +

> +struct __attribute__((aligned (ALIGNMENT)))

>  {

>    char misaligner;

>    int foo[100];
Jakub Jelinek Nov. 22, 2017, 2:03 p.m. UTC | #2 
On Wed, Nov 22, 2017 at 02:51:09PM +0100, Richard Biener wrote:
> On Wed, Nov 22, 2017 at 10:30 AM, Richard Sandiford

> <richard.sandiford@linaro.org> wrote:

> > r254589 was supposed to leave tests unchanged for the default setting

> > of VECTOR_BITS, but I must have got my sums wrong on pr81136.c.

> > Sorry for the breakage.

> >

> > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,

> > OK to install?

> 

> Ok.


That will still FAIL e.g. with -march=skylake-avx512 or -march=knl
(at least when not preferring 256 or 128 bit vectors), those would need
ALIGNMENT 64.

> > 2017-11-22  Richard Sandiford  <richard.sandiford@linaro.org>

> >

> > gcc/testsuite/

> >         PR testsuite/83004

> >         * gcc.dg/vect/pr81136.c: Restore previous alignment of 32

> >         in the default case.

> >

> > Index: gcc/testsuite/gcc.dg/vect/pr81136.c

> > ===================================================================

> > --- gcc/testsuite/gcc.dg/vect/pr81136.c 2017-11-22 09:21:12.538474075 +0000

> > +++ gcc/testsuite/gcc.dg/vect/pr81136.c 2017-11-22 09:28:04.836628929 +0000

> > @@ -2,7 +2,13 @@

> >

> >  #include "tree-vect.h"

> >

> > -struct __attribute__((aligned (VECTOR_BITS / 8)))

> > +#if VECTOR_BITS > 256

> > +#define ALIGNMENT (VECTOR_BITS / 8)

> > +#else

> > +#define ALIGNMENT 32

> > +#endif

> > +

> > +struct __attribute__((aligned (ALIGNMENT)))

> >  {

> >    char misaligner;

> >    int foo[100];


	Jakub
Richard Sandiford Nov. 22, 2017, 2:17 p.m. UTC | #3 
Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, Nov 22, 2017 at 02:51:09PM +0100, Richard Biener wrote:

>> On Wed, Nov 22, 2017 at 10:30 AM, Richard Sandiford

>> <richard.sandiford@linaro.org> wrote:

>> > r254589 was supposed to leave tests unchanged for the default setting

>> > of VECTOR_BITS, but I must have got my sums wrong on pr81136.c.

>> > Sorry for the breakage.

>> >

>> > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,

>> > OK to install?

>> 

>> Ok.

>

> That will still FAIL e.g. with -march=skylake-avx512 or -march=knl

> (at least when not preferring 256 or 128 bit vectors), those would need

> ALIGNMENT 64.


Yeah, the real fix for AVX512 is to define VECTOR_BITS.  And I'd have
thought even AVX2 would need to define it to get clean results on other
tests.  But the patch that introduced VECTOR_BITS just wasn't supposed
to be changing the default behaviour in the way that I'd accidentally
done here.

Do you know what the vect.exp results are like for 256-bit and 512-bit
vectors on x86_64?  Like I said in the PR, I was surprised we were the
first to hit the need for a macro like VECTOR_BITS.  The results for
-msve-vector-bits=256 and -msve-vector-bits=512 were really poor without
it, since many things were hard-coded to values that made sense for
<= 128-bit (or sometimes <= 256-bit) vectors.

Thanks,
Richard
Jakub Jelinek Nov. 22, 2017, 2:45 p.m. UTC | #4 
On Wed, Nov 22, 2017 at 02:17:59PM +0000, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:

> > On Wed, Nov 22, 2017 at 02:51:09PM +0100, Richard Biener wrote:

> >> On Wed, Nov 22, 2017 at 10:30 AM, Richard Sandiford

> >> <richard.sandiford@linaro.org> wrote:

> >> > r254589 was supposed to leave tests unchanged for the default setting

> >> > of VECTOR_BITS, but I must have got my sums wrong on pr81136.c.

> >> > Sorry for the breakage.

> >> >

> >> > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,

> >> > OK to install?

> >> 

> >> Ok.

> >

> > That will still FAIL e.g. with -march=skylake-avx512 or -march=knl

> > (at least when not preferring 256 or 128 bit vectors), those would need

> > ALIGNMENT 64.

> 

> Yeah, the real fix for AVX512 is to define VECTOR_BITS.  And I'd have

> thought even AVX2 would need to define it to get clean results on other

> tests.  But the patch that introduced VECTOR_BITS just wasn't supposed

> to be changing the default behaviour in the way that I'd accidentally

> done here.

> 

> Do you know what the vect.exp results are like for 256-bit and 512-bit

> vectors on x86_64?  Like I said in the PR, I was surprised we were the


Dunno, but can try it for -march=haswell easily (or we could look up
gcc-testresults).  I think some of the Intel folks are surely testing
regularly with --with-arch=haswell configured compiler, judging from many
bugreports about tests from the testsuite that only fail with -march=haswell.

	Jakub
Richard Biener Nov. 22, 2017, 2:48 p.m. UTC | #5 
On Wed, Nov 22, 2017 at 3:17 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Jakub Jelinek <jakub@redhat.com> writes:

>> On Wed, Nov 22, 2017 at 02:51:09PM +0100, Richard Biener wrote:

>>> On Wed, Nov 22, 2017 at 10:30 AM, Richard Sandiford

>>> <richard.sandiford@linaro.org> wrote:

>>> > r254589 was supposed to leave tests unchanged for the default setting

>>> > of VECTOR_BITS, but I must have got my sums wrong on pr81136.c.

>>> > Sorry for the breakage.

>>> >

>>> > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu,

>>> > OK to install?

>>>

>>> Ok.

>>

>> That will still FAIL e.g. with -march=skylake-avx512 or -march=knl

>> (at least when not preferring 256 or 128 bit vectors), those would need

>> ALIGNMENT 64.

>

> Yeah, the real fix for AVX512 is to define VECTOR_BITS.  And I'd have

> thought even AVX2 would need to define it to get clean results on other

> tests.  But the patch that introduced VECTOR_BITS just wasn't supposed

> to be changing the default behaviour in the way that I'd accidentally

> done here.

>

> Do you know what the vect.exp results are like for 256-bit and 512-bit

> vectors on x86_64?  Like I said in the PR, I was surprised we were the

> first to hit the need for a macro like VECTOR_BITS.  The results for

> -msve-vector-bits=256 and -msve-vector-bits=512 were really poor without

> it, since many things were hard-coded to values that made sense for

> <= 128-bit (or sometimes <= 256-bit) vectors.


IIRC testresuits for -mavx2 were clean (at some point...).  Never checked
-mavx512[bf] as I don't have AVX512 HW conveniently available.

Richard.

> Thanks,

> Richard
diff mbox series

Patch

Index: gcc/testsuite/gcc.dg/vect/pr81136.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/pr81136.c	2017-11-22 09:21:12.538474075 +0000
+++ gcc/testsuite/gcc.dg/vect/pr81136.c	2017-11-22 09:28:04.836628929 +0000
@@ -2,7 +2,13 @@ 
 
 #include "tree-vect.h"
 
-struct __attribute__((aligned (VECTOR_BITS / 8)))
+#if VECTOR_BITS > 256
+#define ALIGNMENT (VECTOR_BITS / 8)
+#else
+#define ALIGNMENT 32
+#endif
+
+struct __attribute__((aligned (ALIGNMENT)))
 {
   char misaligner;
   int foo[100];