| Message ID | 8760a21wzd.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | PR83004: Accidental change to pr81136.c for VECTOR_BITS==128 | expand |
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];
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
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
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
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
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];