Re: [PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics

2019-01-21 Thread Ramana Radhakrishnan 
On 20/01/2019 15:48, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jan 17, 2019 at 03:02:00PM +, 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
>

RE: [PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics

2019-01-20 Thread Tamar Christina 
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  
Sent: Sunday, January 20, 2019 3:48 PM
To: Tamar Christina 
Cc: gcc-patches@gcc.gnu.org; nd ; Ramana Radhakrishnan 
; Richard Earnshaw ; 
ni...@redhat.com; Kyrylo Tkachov 
Subject: Re: [PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics

Hi!

On Thu, Jan 17, 2019 at 03:02:00PM +, 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

Re: [PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics

2019-01-20 Thread Segher Boessenkool 
Hi!

On Thu, Jan 17, 2019 at 03:02:00PM +, 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

Re: [PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics

2019-01-17 Thread Ramana Radhakrishnan 
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  
> 
>   PR target/88850
>   * gcc.target/arm/pr51968.c: Use neon intrinsics.
> 

Ok.

Looks pretty obvious to me.

Ramana

[PATCH][GCC][Arm] Rewrite arm testcase to use intrinsics

2019-01-17 Thread Tamar Christina 
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  

PR target/88850
* gcc.target/arm/pr51968.c: Use neon intrinsics.

-- 
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 
 
-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);
 }
 }