| Message ID | 20170901132338.GA32443@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Testsuite,ARM,AArch64] Enable Dot Product for generic tests for ARM and AArch64 [Patch (7/8)] | expand |
On Fri, Sep 01, 2017 at 02:23:39PM +0100, Tamar Christina wrote: > Hi All, > > This patch enables tests for Dot Product vectorization > in gcc for ARM and AArch64. > > The ARMv8.2-a Dot Product instructions only support 8-bit > element vectorization. > > Dot product is available from ARMv8.2-a and onwards. > > Regtested and bootstrapped on aarch64-none-elf and > arm-none-eabi and no issues. I'm surprised that this worked! It looks like you unconditionally add the -march=armv8.2-a+dotprod options, which should cause you to generate instructions which will not execute on targets which don't support this instruction. As far as I can see, this is an execute test, so that should cause undefined instruction exceptions on an Armv8-A target at the very least. So, not OK in its current form. Thanks, James > > Ok for trunk? > > gcc/testsuite > 2017-09-01 Tamar Christina <tamar.christina@arm.com> > > * gcc.dg/vect/vect-reduc-dot-s8a.c > (dg-additional-options, dg-require-effective-target): Add +dotprod. > * gcc.dg/vect/vect-reduc-dot-u8a.c > (dg-additional-options, dg-require-effective-target): Add +dotprod. > > --
> I'm surprised that this worked! > > It looks like you unconditionally add the -march=armv8.2-a+dotprod options, > which should cause you to generate instructions which will not execute on > targets which don't support this instruction. As far as I can see, this is an > execute test, so that should cause undefined instruction exceptions on an > Armv8-A target at the very least. It's not, there is no dg-do specified, which means it defaults to "compile" This is a straight compilation tests that checks to see if the target can do the reduction. There may be a main, but it's never executed, which is why I don't have a hardware check against it. The unconditional armv8.2+dotprod is for this reason. It doesn't matter what hardware. > > So, not OK in its current form. > > Thanks, > James > > > > > Ok for trunk? > > > > gcc/testsuite > > 2017-09-01 Tamar Christina <tamar.christina@arm.com> > > > > * gcc.dg/vect/vect-reduc-dot-s8a.c > > (dg-additional-options, dg-require-effective-target): Add +dotprod. > > * gcc.dg/vect/vect-reduc-dot-u8a.c > > (dg-additional-options, dg-require-effective-target): Add +dotprod. > > > > --
Hi All,
this is a respin with the changes suggested. Note that this patch is no 8/8 in the series.
Regtested on arm-none-eabi, armeb-none-eabi,
aarch64-none-elf and aarch64_be-none-elf with no issues found.
Ok for trunk?
gcc/testsuite
2017-10-06 Tamar Christina <tamar.christina@arm.com>
* gcc.dg/vect/vect-reduc-dot-s8a.c
(dg-additional-options, dg-require-effective-target): Add +dotprod.
* gcc.dg/vect/vect-reduc-dot-u8a.c
(dg-additional-options, dg-require-effective-target): Add +dotprod.
On 06/10/17 13:45, Tamar Christina wrote: > Hi All, > > this is a respin with the changes suggested. Note that this patch is no 8/8 in the series. > > Regtested on arm-none-eabi, armeb-none-eabi, > aarch64-none-elf and aarch64_be-none-elf with no issues found. > > Ok for trunk? > > gcc/testsuite > 2017-10-06 Tamar Christina <tamar.christina@arm.com> > > * gcc.dg/vect/vect-reduc-dot-s8a.c > (dg-additional-options, dg-require-effective-target): Add +dotprod. > * gcc.dg/vect/vect-reduc-dot-u8a.c > (dg-additional-options, dg-require-effective-target): Add +dotprod. > ________________________________________ > From: Tamar Christina > Sent: Monday, September 4, 2017 12:35:39 PM > To: James Greenhalgh > Cc: gcc-patches@gcc.gnu.org; nd; Richard Earnshaw; Marcus Shawcroft > Subject: RE: [PATCH][GCC][Testsuite][ARM][AArch64] Enable Dot Product for generic tests for ARM and AArch64 [Patch (7/8)] > >> I'm surprised that this worked! >> >> It looks like you unconditionally add the -march=armv8.2-a+dotprod options, >> which should cause you to generate instructions which will not execute on >> targets which don't support this instruction. As far as I can see, this is an >> execute test, so that should cause undefined instruction exceptions on an >> Armv8-A target at the very least. > > It's not, there is no dg-do specified, which means it defaults to "compile" > This is a straight compilation tests that checks to see if the target can do the > reduction. There may be a main, but it's never executed, which is why I don't > have a hardware check against it. > > The unconditional armv8.2+dotprod is for this reason. It doesn't matter what hardware. > >> >> So, not OK in its current form. >> >> Thanks, >> James >> >>> >>> Ok for trunk? >>> >>> gcc/testsuite >>> 2017-09-01 Tamar Christina <tamar.christina@arm.com> >>> >>> * gcc.dg/vect/vect-reduc-dot-s8a.c >>> (dg-additional-options, dg-require-effective-target): Add +dotprod. >>> * gcc.dg/vect/vect-reduc-dot-u8a.c >>> (dg-additional-options, dg-require-effective-target): Add +dotprod. >>> >>> -- > iff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c index dc4f52019d5435edbbc811b73dee0f98ff44c1b1..acb6862f8274fb954f69bd45e8edeedcdca4cbf7 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c @@ -1,4 +1,7 @@ /* { dg-require-effective-target vect_int } */ +/* { dg-require-effective-target arm_v8_2a_dotprod_neon_hw { target { aarch64*-*-* || arm*-*-* } } } */ Why do you need hardware with dot-product if these are compile-only tests? (presumably that's what the _hw at the end of the require means). R.
> -----Original Message----- > From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com] > Sent: 12 October 2017 14:21 > To: Tamar Christina; James Greenhalgh > Cc: gcc-patches@gcc.gnu.org; nd; Marcus Shawcroft > Subject: Re: [PATCH][GCC][Testsuite][ARM][AArch64] Enable Dot Product > for generic tests for ARM and AArch64 [Patch (7/8)] > > On 06/10/17 13:45, Tamar Christina wrote: > > Hi All, > > > > this is a respin with the changes suggested. Note that this patch is no 8/8 in > the series. > > > > Regtested on arm-none-eabi, armeb-none-eabi, aarch64-none-elf and > > aarch64_be-none-elf with no issues found. > > > > Ok for trunk? > > > > gcc/testsuite > > 2017-10-06 Tamar Christina <tamar.christina@arm.com> > > > > * gcc.dg/vect/vect-reduc-dot-s8a.c > > (dg-additional-options, dg-require-effective-target): Add +dotprod. > > * gcc.dg/vect/vect-reduc-dot-u8a.c > > (dg-additional-options, dg-require-effective-target): Add +dotprod. > > ________________________________________ > > From: Tamar Christina > > Sent: Monday, September 4, 2017 12:35:39 PM > > To: James Greenhalgh > > Cc: gcc-patches@gcc.gnu.org; nd; Richard Earnshaw; Marcus Shawcroft > > Subject: RE: [PATCH][GCC][Testsuite][ARM][AArch64] Enable Dot Product > > for generic tests for ARM and AArch64 [Patch (7/8)] > > > >> I'm surprised that this worked! > >> > >> It looks like you unconditionally add the -march=armv8.2-a+dotprod > >> options, which should cause you to generate instructions which will > >> not execute on targets which don't support this instruction. As far > >> as I can see, this is an execute test, so that should cause undefined > >> instruction exceptions on an Armv8-A target at the very least. > > > > It's not, there is no dg-do specified, which means it defaults to "compile" > > This is a straight compilation tests that checks to see if the target > > can do the reduction. There may be a main, but it's never executed, > > which is why I don't have a hardware check against it. > > > > The unconditional armv8.2+dotprod is for this reason. It doesn't matter > what hardware. > > > >> > >> So, not OK in its current form. > >> > >> Thanks, > >> James > >> > >>> > >>> Ok for trunk? > >>> > >>> gcc/testsuite > >>> 2017-09-01 Tamar Christina <tamar.christina@arm.com> > >>> > >>> * gcc.dg/vect/vect-reduc-dot-s8a.c > >>> (dg-additional-options, dg-require-effective-target): Add +dotprod. > >>> * gcc.dg/vect/vect-reduc-dot-u8a.c > >>> (dg-additional-options, dg-require-effective-target): Add +dotprod. > >>> > >>> -- > > > > iff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > index > dc4f52019d5435edbbc811b73dee0f98ff44c1b1..acb6862f8274fb954f69bd45e8 > edeedcdca4cbf7 > 100644 > --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > @@ -1,4 +1,7 @@ > /* { dg-require-effective-target vect_int } */ > +/* { dg-require-effective-target arm_v8_2a_dotprod_neon_hw { target { > aarch64*-*-* || arm*-*-* } } } */ > > Why do you need hardware with dot-product if these are compile-only > tests? (presumably that's what the _hw at the end of the require means). James was right in that vect.exp overrides the default from compile to run for these tests, So they are execution tests. > > R.
Ping
On Fri, Oct 06, 2017 at 01:45:15PM +0100, Tamar Christina wrote: > Hi All, > > this is a respin with the changes suggested. Note that this patch is no 8/8 in the series. > > Regtested on arm-none-eabi, armeb-none-eabi, > aarch64-none-elf and aarch64_be-none-elf with no issues found. > > Ok for trunk? > > gcc/testsuite > 2017-10-06 Tamar Christina <tamar.christina@arm.com> > > * gcc.dg/vect/vect-reduc-dot-s8a.c > (dg-additional-options, dg-require-effective-target): Add +dotprod. > * gcc.dg/vect/vect-reduc-dot-u8a.c > (dg-additional-options, dg-require-effective-target): Add +dotprod. > diff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > index dc4f52019d5435edbbc811b73dee0f98ff44c1b1..acb6862f8274fb954f69bd45e8edeedcdca4cbf7 100644 > --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c > @@ -1,4 +1,7 @@ > /* { dg-require-effective-target vect_int } */ > +/* { dg-require-effective-target arm_v8_2a_dotprod_neon_hw { target { aarch64*-*-* || arm*-*-* } } } */ > +/* { dg-additional-options "-O3 -march=armv8.2-a+dotprod" { target { aarch64*-*-* || arm*-*-* } } } */ Drop the -O3, should this one only apply for AArch64? > +/* { dg-add-options arm_v8_2a_dotprod_neon } */ > > #include <stdarg.h> > #include "tree-vect.h" > diff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c > index f3cc6c78c25305d91becd585be8949514ebc521c..c23fe5df252b94d6722708096b8aba7edd100f1a 100644 > --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c > +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c > @@ -1,4 +1,7 @@ > /* { dg-require-effective-target vect_int } */ > +/* { dg-require-effective-target arm_v8_2a_dotprod_neon_hw { target { aarch64*-*-* || arm*-*-* } } } */ > +/* { dg-additional-options "-O3 -march=armv8.2-a+dotprod" { target { aarch64*-*-* || arm*-*-* } } } */ Drop the -O3, should this one only apply for AArch64? Likewise here. Otherwise, this is OK for trunk. Reviewed By: <james.greenhalgh@arm.com> Thanks, James
diff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c index dc4f52019d5435edbbc811b73dee0f98ff44c1b1..c36fbcbf4693f59c2ca747aeb2d41dcd0f48f673 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-s8a.c @@ -1,4 +1,6 @@ /* { dg-require-effective-target vect_int } */ +/* { dg-additional-options "-O3 -march=armv8.2-a+dotprod" { target { aarch64*-*-* || arm*-*-* } } } */ +/* { dg-require-effective-target arm_arch_v8a_ok { target arm*-*-* } } */ #include <stdarg.h> #include "tree-vect.h" diff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c index f3cc6c78c25305d91becd585be8949514ebc521c..c449103d8c8ed8d0861c7e9c231558c86d4f1b85 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u8a.c @@ -1,4 +1,6 @@ /* { dg-require-effective-target vect_int } */ +/* { dg-additional-options "-O3 -march=armv8.2-a+dotprod" { target { aarch64*-*-* || arm*-*-* } } } */ +/* { dg-require-effective-target arm_arch_v8a_ok { target arm*-*-* } } */ #include <stdarg.h> #include "tree-vect.h"