| Message ID | 20171114155452.GA29143@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [AArch64] Add negative tests for dotprod and set minimum version to v8.2 in the target bit. | expand |
On Tue, Nov 14, 2017 at 03:54:56PM +0000, Tamar Christina wrote: > Hi All, > > Dot Product is intended to only be available for Armv8.2-a and newer. > While this restriction is reflected in the intrinsics, the patterns > themselves were missing the Armv8.2-a bit. > > This means that using -march=armv8.1-a+dotprod incorrectly got the > auto-vectorizer to generate dot product instructions. > > Regtested on aarch64-none-elf and no issues. > > Ok for trunk? What is the design here? That -march=armv8.1-a+dotprod is valid, but that the +dotprod is silently ignored? Is that really less surprising than a hard error? How does this interact with -march=native ? If I remember right, we don't get any good indication from the kernel that we are Armv8.2-A rather than Armv8.1-A? Maybe I'm misremembering. Thanks, James > > Thanks, > Tamar > > gcc/ > 2017-11-14 Tamar Christina <tamar.christina@arm.com> > > * config/aarch64/aarch64.h (TARGET_DOTPROD): Add AARCH64_ISA_V8_2. > > gcc/testsuite/ > 2017-11-14 Tamar Christina <tamar.christina@arm.com> > > * gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_1.c: New. > * gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_2.c: New.
The 11/17/2017 22:06, James Greenhalgh wrote: > On Tue, Nov 14, 2017 at 03:54:56PM +0000, Tamar Christina wrote: > > Hi All, > > > > Dot Product is intended to only be available for Armv8.2-a and newer. > > While this restriction is reflected in the intrinsics, the patterns > > themselves were missing the Armv8.2-a bit. > > > > This means that using -march=armv8.1-a+dotprod incorrectly got the > > auto-vectorizer to generate dot product instructions. > > > > Regtested on aarch64-none-elf and no issues. > > > > Ok for trunk? > > What is the design here? That -march=armv8.1-a+dotprod is valid, but that > the +dotprod is silently ignored? Is that really less surprising than > a hard error? As far as I am aware, AArch64 unlike ARM does not provide a facility to reject invalid combinations of architecture and extensions. The argument parsers don't specify any relationship between architectures and extensions. This is why -march=armv8-a+fp16 is also accepted silently. This patch merely makes it not generate code that can't possibly run. > > How does this interact with -march=native ? If I remember right, we don't > get any good indication from the kernel that we are Armv8.2-A rather than > Armv8.1-A? Maybe I'm misremembering. As far as I am aware, if you have a kernel which has asimddp support, and you compile with -march=armv8.1-a then it's equivalent to specifying -march=armv8.1-a+dotprod, as in, the bit will be enabled, but the feature won't, as it's gated on Armv8.2-a (with this patch). > > Thanks, > James > > > > > Thanks, > > Tamar > > > > gcc/ > > 2017-11-14 Tamar Christina <tamar.christina@arm.com> > > > > * config/aarch64/aarch64.h (TARGET_DOTPROD): Add AARCH64_ISA_V8_2. > > > > gcc/testsuite/ > > 2017-11-14 Tamar Christina <tamar.christina@arm.com> > > > > * gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_1.c: New. > > * gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_2.c: New. --
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 93d29b84d47b7017661a2129d61e7d740bbf7c93..b6805775079626417e05f6f5a74289670330243d 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -192,7 +192,7 @@ extern unsigned aarch64_architecture_version; #define TARGET_SIMD_F16INST (TARGET_SIMD && AARCH64_ISA_F16) /* Dot Product is an optional extension to AdvSIMD enabled through +dotprod. */ -#define TARGET_DOTPROD (TARGET_SIMD && AARCH64_ISA_DOTPROD) +#define TARGET_DOTPROD (TARGET_SIMD && AARCH64_ISA_DOTPROD && AARCH64_ISA_V8_2) /* ARMv8.3-A features. */ #define TARGET_ARMV8_3 (AARCH64_ISA_V8_3) diff --git a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_1.c b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_1.c new file mode 100644 index 0000000000000000000000000000000000000000..3a1664b0740fa6fb32f99f3cc40025afce2292e9 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_1.c @@ -0,0 +1,9 @@ +/* { dg-do compile { target { aarch64*-*-* } } } */ +/* { dg-additional-options "-O3 -march=armv8.1-a+dotprod" } */ + +#define N 64 +#define TYPE signed + +#include "vect-dot-qi.h" + +/* { dg-final { scan-assembler-not {sdot\tv[0-9]+\.4s, v[0-9]+\.16b, v[0-9]+\.16b} } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_2.c b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_2.c new file mode 100644 index 0000000000000000000000000000000000000000..f8c746cb094194c74bb280bf68a38dd435c46c64 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vect-dot-s8_2.c @@ -0,0 +1,9 @@ +/* { dg-do compile { target { aarch64*-*-* } } } */ +/* { dg-additional-options "-O3 -march=armv8.3-a+dotprod" } */ + +#define N 64 +#define TYPE signed + +#include "vect-dot-qi.h" + +/* { dg-final { scan-assembler-times {sdot\tv[0-9]+\.4s, v[0-9]+\.16b, v[0-9]+\.16b} 4 } } */