[PATCH 1/2] [ARM] PR68532: Fix up vuzp for big endian

[Charles Baylis]

On 8 February 2016 at 11:42, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote:
> Hi Charles,
>
>
> On 03/02/16 18:59, charles.baylis@linaro.org wrote:
>>

>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -28208,6 +28208,35 @@ arm_expand_vec_perm (rtx target, rtx op0, rtx
>> op1, rtx sel)
>>     arm_expand_vec_perm_1 (target, op0, op1, sel);
>>   }
>>   +/* map lane ordering between architectural lane order, and GCC lane
>> order,
>> +   taking into account ABI.  See comment above output_move_neon for
>> details.  */
>> +static int
>> +neon_endian_lane_map (machine_mode mode, int lane)
>
>
> s/map/Map/
> New line between comment and function signature.

Done.

>> +{
>> +  if (BYTES_BIG_ENDIAN)
>> +  {
>> +    int nelems = GET_MODE_NUNITS (mode);
>> +    /* Reverse lane order.  */
>> +    lane = (nelems - 1 - lane);
>> +    /* Reverse D register order, to match ABI.  */
>> +    if (GET_MODE_SIZE (mode) == 16)
>> +      lane = lane ^ (nelems / 2);
>> +  }
>> +  return lane;
>> +}
>> +
>> +/* some permutations index into pairs of vectors, this is a helper
>> function
>> +   to map indexes into those pairs of vectors.  */
>> +static int
>> +neon_pair_endian_lane_map (machine_mode mode, int lane)
>
>
> Similarly, s/some/Some/ and new line after comment.

Done.

>> +{
>> +  int nelem = GET_MODE_NUNITS (mode);
>> +  if (BYTES_BIG_ENDIAN)
>> +    lane =
>> +      neon_endian_lane_map (mode, lane & (nelem - 1)) + (lane & nelem);
>> +  return lane;
>> +}
>> +
>>   /* Generate or test for an insn that supports a constant permutation.
>> */
>>     /* Recognize patterns for the VUZP insns.  */
>> @@ -28218,14 +28247,22 @@ arm_evpc_neon_vuzp (struct expand_vec_perm_d *d)
>>     unsigned int i, odd, mask, nelt = d->nelt;
>>     rtx out0, out1, in0, in1;
>>     rtx (*gen)(rtx, rtx, rtx, rtx);
>> +  int first_elem;
>> +  int swap;
>>
>
> Just make this a bool.

As discussed on IRC, this variable does contain an integer. I have
renamed it as swap_nelt, and changed the test on it below.

[snip]

>>   @@ -28258,10 +28296,9 @@ arm_evpc_neon_vuzp (struct expand_vec_perm_d
>> *d)
>>       in0 = d->op0;
>>     in1 = d->op1;
>> -  if (BYTES_BIG_ENDIAN)
>> +  if (swap)
>>       {
>>         std::swap (in0, in1);
>> -      odd = !odd;
>>       }
>
> remove the braces around the std::swap

Done. Also changed if (swap) to if (swap_nelt != 0)

[snip]

>> @@ -0,0 +1,24 @@
>> +/* { dg-options "-O2 -ftree-vectorize -fno-vect-cost-model" } */
>> +
>> +#define SIZE 128
>> +unsigned short _Alignas (16) in[SIZE];
>> +
>> +extern void abort (void);
>> +
>> +__attribute__ ((noinline)) int
>> +test (unsigned short sum, unsigned short *in, int x)
>> +{
>> +  for (int j = 0; j < SIZE; j += 8)
>> +    sum += in[j] * x;
>> +  return sum;
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  for (int i = 0; i < SIZE; i++)
>> +    in[i] = i;
>> +  if (test (0, in, 1) != 960)
>> +    abort ();
>
>
> AFAIK tests here usually prefer __builtin_abort ();
> That way you don't have to declare the abort prototype in the beginning.

Done.

Updated patch attached
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-PR68532-Fix-up-vuzp-for-big-endian.patch
Type: text/x-diff
Size: 9836 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160209/d2946a64/attachment.bin>