On Fri, Nov 19, 2021 at 11:20 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Thu, Nov 11, 2021 at 3:13 PM Philipp Tomsich
> <philipp.toms...@vrull.eu> wrote:
> >
> > The RISC-V Zbb extension adds an XLEN (i.e. SImode for rv32, DImode
> > for rv64) bswap instruction (rev8). While, with the current master,
> > SImode is synthesized correctly from DImode, HImode is not.
> >
> > This change adds an appropriate expansion for a HImode bswap, if a
> > wider bswap is available.
> >
> > Without this change, the following rv64gc_zbb code is generated for
> > __builtin_bswap16():
> > slliw a5,a0,8
> > zext.h a0,a0
> > srliw a0,a0,8
> > or a0,a5,a0
> > sext.h a0,a0 // this is a 16bit sign-extension following
> > // the byteswap (e.g. on a 'short' function
> > // return).
> >
> > After this change, a bswap (rev8) is used and any extensions are
> > combined into the shift-right:
> > rev8 a0,a0
> > srai a0,a0,48 // the sign-extension is combined into the
> > // shift; a srli is emitted otherwise...
> >
> > gcc/ChangeLog:
> >
> > * optabs.c (expand_unop): support expanding a HImode bswap
> > using SImode or DImode, followed by a shift.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/zbb-bswap.c: New test.
> >
> > Signed-off-by: Philipp Tomsich <philipp.toms...@vrull.eu>
> > ---
> >
> > gcc/optabs.c | 6 ++++++
> > gcc/testsuite/gcc.target/riscv/zbb-bswap.c | 22 ++++++++++++++++++++++
> > 2 files changed, 28 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.target/riscv/zbb-bswap.c
> >
> > diff --git a/gcc/optabs.c b/gcc/optabs.c
> > index 019bbb62882..7a3ffbe4525 100644
> > --- a/gcc/optabs.c
> > +++ b/gcc/optabs.c
> > @@ -3307,6 +3307,12 @@ expand_unop (machine_mode mode, optab unoptab, rtx
> > op0, rtx target,
> > return temp;
> > }
> >
> > + /* If we are missing a HImode BSWAP, but have one for SImode or
> > + DImode, use a BSWAP followed by a SHIFT. */
> > + temp = widen_bswap (as_a <scalar_int_mode> (mode), op0, target);
> > + if (temp)
> > + return temp;
> > +
>
> I think it would be more natural to temporarily terminate the HImode case here
> and re-open it inside the following
>
> if (is_a <scalar_int_mode> (mode, &int_mode))
> {
> temp = widen_bswap (int_mode, op0, target);
> if (temp)
> return temp;
>
> here to handle the ashl/lshr/ior fallback.
But as Kito says, code generation for more targets would need to be looked at.
Richard.
> Richard.
>
> > last = get_last_insn ();
> >
> > temp1 = expand_binop (mode, ashl_optab, op0,
> > diff --git a/gcc/testsuite/gcc.target/riscv/zbb-bswap.c
> > b/gcc/testsuite/gcc.target/riscv/zbb-bswap.c
> > new file mode 100644
> > index 00000000000..6ee27d9f47a
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/zbb-bswap.c
> > @@ -0,0 +1,22 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_zbb -mabi=lp64 -O2" } */
> > +
> > +unsigned long
> > +func64 (unsigned long i)
> > +{
> > + return __builtin_bswap64(i);
> > +}
> > +
> > +unsigned int
> > +func32 (unsigned int i)
> > +{
> > + return __builtin_bswap32(i);
> > +}
> > +
> > +unsigned short
> > +func16 (unsigned short i)
> > +{
> > + return __builtin_bswap16(i);
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "rev8" 3 } } */
> > --
> > 2.32.0
> >
