On 20 June 2012 12:29, Julian Brown <jul...@codesourcery.com> wrote:
> On Wed, 20 Jun 2012 11:56:39 +0100
> Ramana Radhakrishnan <ramana.radhakrish...@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
