]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
RISC-V: Expose bswapsi for TARGET_64BIT
authorJeff Law <jlaw@ventanamicro.com>
Tue, 5 Sep 2023 21:39:16 +0000 (15:39 -0600)
committerJeff Law <jlaw@ventanamicro.com>
Tue, 5 Sep 2023 21:39:16 +0000 (15:39 -0600)
Various bswapsi tests are failing for rv64.  More importantly, we're generating
crappy code.

Let's take the first test from bswapsi-1.c as an example.

> typedef unsigned int uint32_t;
>
> #define __const_swab32(x) ((uint32_t)(                                \
>         (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 24) |            \
>         (((uint32_t)(x) & (uint32_t)0x0000ff00UL) <<  8) |            \
>         (((uint32_t)(x) & (uint32_t)0x00ff0000UL) >>  8) |            \
>         (((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24)))
>
> /* This byte swap implementation is used by the Linux kernel and the
>    GNU C library.  */
>
> uint32_t
> swap32_a (uint32_t in)
> {
>   return __const_swab32 (in);
> }
>
>
>

We generate this for rv64gc_zba_zbb_zbs:

>         srliw   a1,a0,24
>         slliw   a5,a0,24
>         slliw   a3,a0,8
>         li      a2,16711680
>         li      a4,65536
>         or      a5,a5,a1
>         and     a3,a3,a2
>         addi    a4,a4,-256
>         srliw   a0,a0,8
>         or      a5,a5,a3
>         and     a0,a0,a4
>         or      a0,a5,a0
>         retUrgh!

After this patch we generate:

>         rev8    a0,a0
>         srai    a0,a0,32
>         ret
Clearly better.

The stated rationale behind not exposing bswapsi2 for TARGET_64BIT is that the
RTL expanders already know how to widen a bswap, which is definitely true.  But
it's the case that failure to expose a bswapsi will cause the 32bit bswap
optimizations in gimple store merging to not trigger.  Thus we get crappy code.

To fix this we expose bswapsi on TARGET_64BIT.  gimple-store-merging then
detects the 32bit bswap idioms and generates suitable __builtin calls.  The
expander will "FAIL" expansion for TARGET_64BIT which forces the generic
expander code to synthesize the operation (we could synthesize in here, but
that'd result in duplicate code).

Tested on rv64gc_zba_zbb_zbs, fixes all the bswapsi failures in the testsuite
without any regressions.

gcc/
* config/riscv/bitmanip.md (bswapsi2): Expose for TARGET_64BIT.

gcc/config/riscv/bitmanip.md

index 7b55528ee49af2aa064ea03dd8d7b5ccceedcd5d..1544ef4e1252126b3eb94dec76319ae197b19e00 100644 (file)
 (define_expand "bswapsi2"
   [(set (match_operand:SI 0 "register_operand")
        (bswap:SI (match_operand:SI 1 "register_operand")))]
-  "(!TARGET_64BIT && (TARGET_ZBB || TARGET_ZBKB)) || TARGET_XTHEADBB")
+  "TARGET_ZBB || TARGET_ZBKB || TARGET_XTHEADBB"
+{
+  /* Expose bswapsi2 on TARGET_64BIT so that the gimple store
+     merging pass will create suitable bswap insns.  We can actually
+     just FAIL that case when generating RTL and let the generic code
+     handle it.  */
+  if (TARGET_64BIT && !TARGET_XTHEADBB)
+    FAIL;
+})
+
 
 (define_insn "*bswap<mode>2"
   [(set (match_operand:X 0 "register_operand" "=r")