[Patch ARM] implement bswap16

Christophe Lyon christophe.lyon@linaro.org
Thu Sep 6 16:07:00 GMT 2012


On 6 September 2012 10:48, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 05/09/12 17:01, Christophe Lyon wrote:
>
> +(define_insn "*arm_revsh"
> +  [(set (match_operand:SI 0 "s_register_operand" "=r")
> +       (sign_extend:SI (bswap:HI (match_operand:HI 1 "s_register_operand" "r"))))]
> +  "TARGET_32BIT && arm_arch6"
> +  "revsh%?\t%0, %1"
> +  [(set_attr "predicable" "yes")
> +   (set_attr "length" "4")]
>
> Can you add additional constraints for the t1 encoding for this and the other TARGET_32BIT patterns.  Then the compiler will get the length calculations correct.  Something like:
>
>
> (define_insn "*arm_revsh"
> +  [(set (match_operand:SI 0 "s_register_operand" "=l,r")
> +       (sign_extend:SI (bswap:HI (match_operand:HI 1 "s_register_operand" "l,r"))))]
>   "TARGET_32BIT && arm_arch6"
>   "revsh%?\t%0, %1"
>   [(set_attr "predicable" "yes")
> +   (set_attr "arch" "t2,*")
> +   (set_attr "length" "2,4")]
>
> Brownie points for retro-fitting this to the existing rev patterns.

OK I will do it.

But why are the thumb1_XXX patterns still necessary?
I tried removing them, but compiling the testcase with -march=armv6
-mthumb makes the compiler fail (internal compiler error:
output_operand: invalid %-code)

> +(define_expand "bswaphi2"
> +  [(set (match_operand:HI 0 "s_register_operand" "=r")
> +       (bswap:HI (match_operand:HI 1 "s_register_operand" "r")))]
>
> Define_expand doesn't take constraints.
Oops. So I'll also have to clean bswapsi2 :-)

> Finally, these patterns should be grouped with the other byte-reversal patterns in arm.md, not placed at the end of the file.
I am not sure to understand: I added them right after bswapsi2, do
they need to be before it?

Christophe.



More information about the Gcc-patches mailing list