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) > +(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.
