On 06/09/12 17:07, Christophe Lyon wrote:
> On 6 September 2012 10:48, Richard Earnshaw <rearn...@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.
