| Message ID | 20190117150157.GA10148@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Arm] Rewrite arm testcase to use intrinsics | expand |
On 17/01/2019 15:02, Tamar Christina wrote: > Hi All, > > This test was added back when builtins were being used instead of ACLE > intrinsics. The test as far as I can tell is really testing vcombine, > however some of these builtins no longer exist and causes an ICE. > > This fixes the testcase by changing it to use neon intrinsics. > > Regtested on arm-none-eabi and no issues. > > Ok for trunk? > > Thanks, > Tamar > > gcc/testsuite/ChangeLog: > > 2019-01-17 Tamar Christina <tamar.christina@arm.com> > > PR target/88850 > * gcc.target/arm/pr51968.c: Use neon intrinsics. > Ok. Looks pretty obvious to me. Ramana
Hi! On Thu, Jan 17, 2019 at 03:02:00PM +0000, Tamar Christina wrote: > This test was added back when builtins were being used instead of ACLE > intrinsics. The test as far as I can tell is really testing vcombine, > however some of these builtins no longer exist and causes an ICE. > > This fixes the testcase by changing it to use neon intrinsics. Shouldn't the ICE be fixed as well? [ Sorry if you send a separate patch for that and I missed it ]. Segher
Hi Segher, Yes, that's why the PR is still open 😊 The ICE can be reproduced with a much simpler testcase which is the one we use for repro in the PR, which is now a P1. The reason for changing this testcase was that it was invalid code. Regards, Tamar -----Original Message----- From: Segher Boessenkool <segher@kernel.crashing.org> Sent: Sunday, January 20, 2019 3:48 PM To: Tamar Christina <Tamar.Christina@arm.com> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; nickc@redhat.com; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> Subject: Re: [PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics Hi! On Thu, Jan 17, 2019 at 03:02:00PM +0000, Tamar Christina wrote: > This test was added back when builtins were being used instead of ACLE > intrinsics. The test as far as I can tell is really testing vcombine, > however some of these builtins no longer exist and causes an ICE. > > This fixes the testcase by changing it to use neon intrinsics. Shouldn't the ICE be fixed as well? [ Sorry if you send a separate patch for that and I missed it ]. Segher
On 20/01/2019 15:48, Segher Boessenkool wrote: > Hi! > > On Thu, Jan 17, 2019 at 03:02:00PM +0000, Tamar Christina wrote: >> This test was added back when builtins were being used instead of ACLE >> intrinsics. The test as far as I can tell is really testing vcombine, >> however some of these builtins no longer exist and causes an ICE. >> >> This fixes the testcase by changing it to use neon intrinsics. JFTR, I think this was a case when we were using builtins for implementation of the ACLE intrinsics and the testcase was reduced to remove the use of arm_neon.h . Thus the test needs to go back to using the neon intrinsics directly if possible. Also if there are other tests like this it would be a good small cleanup to do. > > Shouldn't the ICE be fixed as well? [ Sorry if you send a separate patch > for that and I missed it ]. > Indeed but that's a separate issue to this. regards Ramana > > Segher >
diff --git a/gcc/testsuite/gcc.target/arm/pr51968.c b/gcc/testsuite/gcc.target/arm/pr51968.c index 99bdb961757bfa62aec5ef1426137425e57898b0..781470223db0d85214bced0b64fda68b4c43967f 100644 --- a/gcc/testsuite/gcc.target/arm/pr51968.c +++ b/gcc/testsuite/gcc.target/arm/pr51968.c @@ -1,14 +1,10 @@ /* PR target/51968 */ /* { dg-do compile } */ -/* { dg-options "-O2 -Wno-implicit-function-declaration -march=armv7-a -mfloat-abi=softfp -mfpu=neon" } */ +/* { dg-options "-O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon" } */ /* { dg-require-effective-target arm_neon_ok } */ +#include <arm_neon.h> -typedef __builtin_neon_qi int8x8_t __attribute__ ((__vector_size__ (8))); -typedef __builtin_neon_uqi uint8x8_t __attribute__ ((__vector_size__ (8))); -typedef __builtin_neon_qi int8x16_t __attribute__ ((__vector_size__ (16))); -typedef __builtin_neon_hi int16x8_t __attribute__ ((__vector_size__ (16))); -typedef __builtin_neon_si int32x4_t __attribute__ ((__vector_size__ (16))); -struct T { int8x8_t val[2]; }; +struct T { int8x8x2_t val; }; int y; void @@ -17,16 +13,16 @@ foo (int8x8_t z, int8x8_t x, int16x8_t b, int8x8_t n) if (y) { struct T m; - __builtin_neon_vuzpv8qi (&m.val[0], z, x); + m.val = vuzp_s8 (z, x); } for (;;) { int8x16_t g; int8x8_t h, j, k; struct T m; - j = __builtin_neon_vqmovunv8hi (b); - g = __builtin_neon_vcombinev8qi (j, h); - k = __builtin_neon_vget_lowv16qi (g); - __builtin_neon_vuzpv8qi (&m.val[0], k, n); + j = vqmovn_s16 (b); + g = vcombine_s8 (j, h); + k = vget_low_s8 (g); + m.val = vuzp_s8 (k, n); } }