https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63259
--- Comment #14 from thopre01 at gcc dot gnu.org ---
(In reply to Oleg Endo from comment #13)
> (In reply to thopre01 from comment #12)
> >
> > That's good, it means the pattern is recognized. Is there an optab defined
> > for bswap16?
>
> Nope. Just this:
>
> (define_insn "rotlhi3_8"
> [(set (match_operand:HI 0 "arith_reg_dest" "=r")
> (rotate:HI (match_operand:HI 1 "arith_reg_operand" "r")
> (const_int 8)))]
> "TARGET_SH1"
> "swap.b %1,%0"
> [(set_attr "type" "arith")])
>
> If I remember correctly, there was something that would check whether it can
> use the rotate pattern instead of a bswaphi2.
It's rather the contrary. The bswap pass will replace the statements by a 8 bit
rotation if the value is 16bit and the expander will choose a bswaphi pattern
for that if the backend has one, otherwise it will keep the rotation.
The problem is that currently the bswap pass still bails out if there is no 16
bit bswap available. I shall fix that.
> After adding the following
> pattern:
>
> (define_expand "bswaphi2"
> [(set (match_operand:HI 0 "arith_reg_dest")
> (bswap:HI (match_operand:HI 1 "arith_reg_operand")))]
> "TARGET_SH1"
> {
> if (!can_create_pseudo_p ())
> FAIL;
> else
> {
> emit_insn (gen_rotlhi3_8 (operands[0], operands[1]));
> DONE;
> }
> })
>
> it looks much better. The cases above work, except for the signed short.
You mean with the added bswaphi2 pattern the pattern is still unchanged?
> On SH the bswap:HI HW insn actually doesn't modify the upper 16 bit of the
> 32 bit register. What it does is:
>
> unsigned int test_0999 (unsigned int a, unsigned int b)
> {
> return (a & 0xFFFF0000) | ((a & 0xFF00) >> 8) | ((a & 0xFF) << 8);
> }
>
> I was afraid that using a bswap:HI will result in unnecessary code around it:
>
>
> mov.l .L6,r0 ! r0 = 0xFFFF0000
> and r4,r0
> extu.w r4,r4
> swap.b r4,r4
> rts
> or r4,r0
Looks good to me, what exactly is the problem?
>
> In my case, combine is looking for the pattern:
>
> Failed to match this instruction:
> (set (reg:SI 172 [ D.1356 ])
> (ior:SI (ior:SI (and:SI (ashift:SI (reg/v:SI 170 [ a ])
> (const_int 8 [0x8]))
> (const_int 65280 [0xff00]))
> (zero_extract:SI (reg/v:SI 170 [ a ])
> (const_int 8 [0x8])
> (const_int 8 [0x8])))
> (and:SI (reg/v:SI 170 [ a ])
> (const_int -65536 [0xffffffffffff0000]))))
>
> Which should then work.
If you have a bswap instruction it seems better to define a pattern for that
which the expander will use. That's the job of the bswap pass to detect a
bswap, it shouldn't be done by combine.
>
> >
> > Same as the other pattern, can you try to print n->n in hex with this new
> > limit? My guess is that the pattern is now recognized but fails later for
> > the same reason as above.
>
> With increased limit, the number is/was the same. The bswap:HI expander was
> missing. Thanks!
Ok so the limit ought to be increased and the check for bswaphi removed. I'll
take a stab at that but that will probably make it only after Christmas.
