https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114252
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |sayle at gcc dot gnu.org
--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Georg-Johann Lay from comment #8)
> (In reply to Richard Biener from comment #7)
> > Note I do understand what you are saying, just the middle-end in detecting
> > and using __builtin_bswap32 does what it does everywhere else - it checks
> > whether the target implements the operation.
> >
> > The middle-end doesn't try to actually compare costs (it has no idea of the
> > bswapsi costs),
>
> But even when the bswapsi insn costs nothing, the v14 code has these
> additional 6 movqi insns 32...37 compared to v13 code. In order to have the
> same performance like v13 code, a bswapsi would have to cost negative 6
> insns. And an optimizer that assumes negative costs is not reasonable, in
> particular because the recognition of bswap opportunities serves
> optimization -- or is supposed to serve it as far as I understand.
>
> > and it most definitely doesn't see how AVR is special in
> > having only QImode registers and thus the created SImode load (which the
> > target supports!) will end up as four registers.
>
> Even when the bswap insn would cost nothing the code is worse.
Yes, I know.
> > The only thing that maybe would make sense with AVR exposing bswapsi is
> > users calling __builtin_bswap but since it always expands as a libcall
> > even that makes no sense.
>
> It makes perfect sense when C/C++ code uses __builtin_bswap32:
>
> * With current bswapsi insn, the code does a call that performs SI:22 =
> bswap(SI:22) with NO additionall register pressure.
>
> * Without bswap insn, the code does a real ABI call that performs SI:22 =
> bswap(SI:22) PLUS IT CLOBBERS r18, r19, r20, r21, r26, r27, r30 and r31;
> which are the most powerful GPRs.
I think the target controls the "libcall" ABI that's used for calls to
libgcc, but somehow we fail to go that path (but I can see __bswapsi
and __bswapdi even in the x86_64 libgcc). In particular
OPTAB_NC(bswap_optab, "bswap$a2", BSWAP)
doesn't list bswap as having a libfunc ...
> > So my preferred fix would be to remove bswapsi from avr.md?
>
> Is there a way that the backend can fold a call to an insn that performs
> better that a call? Like in TARGET_FOLD_BUILTIN? As far as I know, the
> backend can only fold target builtins, but not common builtins? Tree fold
> cannot fold to an insn obviously, but it could fold to inline asm, no?
>
> Or can the target change an optabs entry so it expands to an insn that's
> more profitable that a respective call? (like avr.md's bswap insn with
> transparent call is more profitable than a real call).
I think the target should implement an inline bswap, possibly via a
define_insn_and_split or define_split so the byte ops are only exposed
at a desired point; important points being lower_subreg (split-wide-types)
and register allocation - possibly lower_subreg should itself know
how to handle bswap (though the degenerate AVR case is quite special).
I've CCed Roger who might know the traps with "implementing" an SImode
bswap on a target with just QImode regs but multi-reg operations not
decomposed during most of the RTL pipeline(?)
> The avr backend does this for many other stuff, too:
>
> divmod, SI and PSI multiplications, parity, popcount, clz, ffs,
Indeed. Maybe it's never the case that a loop implementing clz is
better than a libcall or separate div/mod are better than divmod
(oddly divmod also lacks the libcall entry for the optabs...).
> > Does it benefit from recognizing bswap done with shifts on an int?
>
> I don't fully understand that question. You mean to write code that shifts
> bytes around like in
> uint32_t res = 0;
> res |= ((uint32_t) buf[0]) << 24;
> res |= ((uint32_t) buf[1]) << 16;
> res |= (uint32_t) buf[2] << 8;
> res |= buf[3];
> return res;
> is better than a bswapsi call?
Yeah. Or comparing to open-coding the bswap without going through the call.
I don't have a AVR libgcc around, but libgcc2.s has
#ifdef L_bswapsi2
SItype
__bswapsi2 (SItype u)
{
return ((((u) & 0xff000000u) >> 24)
| (((u) & 0x00ff0000u) >> 8)
| (((u) & 0x0000ff00u) << 8)
| (((u) & 0x000000ffu) << 24));
}
#endif
and that's compiled to
__bswapsi2:
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
rcall __bswapsi2
/* epilogue start */
ret
so this can't be it ;)
