Re: [Patch ARM] PR51980 / PR49081 Improve Neon permute intrinsics.

[Ramana Radhakrishnan <ramana dot radhakrishnan at linaro dot org>]

On 20 June 2012 12:29, Julian Brown <julian@codesourcery.com> wrote:
> On Wed, 20 Jun 2012 11:56:39 +0100
> Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org> wrote:
>
>> Hi,
>>
>> This patch helps use the __builtin_shuffle intrinsics to implement the
>> Neon permute intrinsics following on from Julian's and my patch last
>> week. It needed support for __builtin_shuffle in the C++ frontend
>> which is now in and has been for the past few days , so I'm a little
>> happier with this going in now.The changes to Julian's ?patch are to
>> drop the "mask" generation and now this directly generates the vector
>> constants instead.
>
> A small stylistic point I noticed: in,
>
> ? let rec print_lines = function
> ? ? [] -> ()
> - ?| [line] -> Format.printf "%s" line
> - ?| line::lines -> Format.printf "%s@," line; print_lines lines in
> + ?| [line] -> if line <> "" then Format.printf "%s" line else ()
> + ?| line::lines -> (if line <> "" then Format.printf "%s@," line);
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? print_lines lines in
> ? print_lines body; close_braceblock ffmt;
> ? end_function ffmt
>
> You can use constant strings in pattern matches, so this can be just:
>
> ?let rec print_lines = function
> ? ?[] | ""::_ -> ()
> ?| [line] -> Format.printf...
> ?| line::lines -> Format.printf...
>
> You didn't need the brackets () around the if, btw. It's semantically
> quite like C: only a single statement after the "then" is conditional.
> If you want multiple statements conditionalised, the idiomatic
> way to do it is use begin...end (equivalent to { } in C) after the then
> keyword.
>
> HTH,

Thanks that helps , I'll wait for a couple of days for any more
comments on this.

Ramana

>
> Julian