[Patch ARM] implement bswap16

[Richard Earnshaw]

On 06/09/12 17:07, Christophe Lyon wrote:
> 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)
> 

They probably aren't necessary.  It should be possible to combine these
patterns into

(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"))))]
  "arm_arch6"
  "revsh%?\t%0, %1"
  [(set_attr "predicable" "yes")
   (set_attr "arch" "t,32")
   (set_attr "length" "2,4")]


>> +(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?
> 

Strike that bit, then.  I saw the reference to "include "ldmstm.md"" in
the tail of your patch and hadn't realised that the bswap patterns were
already immediately before that.