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

[Charles Baylis <charles dot baylis at linaro dot org>]

On 9 February 2016 at 17:08, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote:
>
> On 09/02/16 17:00, Charles Baylis wrote:
>>
>> 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.
>
>
> This is ok.

Thanks. Committed to trunk as r233251