On Fri, Jun 05, 2026 at 02:06:44PM +0100, Richard Sandiford wrote:
> Yeah, agreed. Namely:
>
> (define_expand "bswapsi2"
> [(set (match_operand:SI 0 "register_operand")
> (bswap:SI (match_operand:SI 1 "register_operand")))]
> "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;
> })
>
> I don't think that's a good enough justification for an exception
> like this. Even a target hook that says "is bswap cheap?" would IMO
> be better than the above (and could default to "is bswap implemented?").
> But hopefully the hook could be generalised a bit. For example, maybe
> the hook could be a wrapper around:
>
> /* Return true if we can implement OP for mode MODE directly, without
> resorting
> to a libfunc. This usually means that OP will be implemented inline.
>
> Note that this function cannot tell whether the target pattern chooses to
> use libfuncs internally. */
>
> bool
> can_open_code_p (optab op, machine_mode mode)
>
> and additionally test for "cheapness".
In this particular case it is about widen_bswap_or_bitreverse (or e.g. for
clz/clrsb also for widen_leading).
Now, store merging already has hacks which consider
expand_doubleword_bswap_or_bitreverse implementing for 32-bit targets
bswapdi2 using 2 bswapsi2 expanders, so I think we can do the same for
bswapsi2 using bswapdi2 and right shift.
I don't have setup to test this on riscv, but can test on x86_64/i686.
2026-06-05 Jakub Jelinek <[email protected]>
* gimple-ssa-store-merging.cc (maybe_optimize_vector_constructor):
Also support bswap32 if bswapdi2 expander is present. Add comment
about bswap64 support on 32-bit targets with bswapsi2 expander.
(pass_optimize_bswap::execute): Likewise.
* config/riscv/bitmanip.md (bswapsi2): Change condition to disable
the expander on TARGET_64BIT without TARGET_XTHEADBB, never FAIL.
--- gcc/gimple-ssa-store-merging.cc.jj 2026-05-30 17:45:09.430109204 +0200
+++ gcc/gimple-ssa-store-merging.cc 2026-06-05 15:30:14.769415035 +0200
@@ -1570,7 +1570,10 @@ maybe_optimize_vector_constructor (gimpl
break;
case 32:
if (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
- && optab_handler (bswap_optab, SImode) != CODE_FOR_nothing)
+ && (optab_handler (bswap_optab, SImode) != CODE_FOR_nothing
+ /* widen_bswap_or_bitreverse can implement 32-bit bswap
+ using bswapdi2 and shift. */
+ || optab_handler (bswap_optab, DImode) != CODE_FOR_nothing))
{
load_type = uint32_type_node;
fndecl = builtin_decl_explicit (BUILT_IN_BSWAP32);
@@ -1582,6 +1585,8 @@ maybe_optimize_vector_constructor (gimpl
case 64:
if (builtin_decl_explicit_p (BUILT_IN_BSWAP64)
&& (optab_handler (bswap_optab, DImode) != CODE_FOR_nothing
+ /* expand_doubleword_bswap_or_bitreverse can use 2 bswapsi2
+ expanders on 32-bit targets. */
|| (word_mode == SImode
&& builtin_decl_explicit_p (BUILT_IN_BSWAP32)
&& optab_handler (bswap_optab, SImode) != CODE_FOR_nothing)))
@@ -1631,9 +1636,15 @@ pass_optimize_bswap::execute (function *
tree bswap32_type = NULL_TREE, bswap64_type = NULL_TREE;
bswap32_p = (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
- && optab_handler (bswap_optab, SImode) != CODE_FOR_nothing);
+ && (optab_handler (bswap_optab, SImode) != CODE_FOR_nothing
+ /* widen_bswap_or_bitreverse can implement 32-bit bswap
+ using bswapdi2 and shift. */
+ || (optab_handler (bswap_optab, DImode)
+ != CODE_FOR_nothing)));
bswap64_p = (builtin_decl_explicit_p (BUILT_IN_BSWAP64)
&& (optab_handler (bswap_optab, DImode) != CODE_FOR_nothing
+ /* expand_doubleword_bswap_or_bitreverse can use 2 bswapsi2
+ expanders on 32-bit targets. */
|| (bswap32_p && word_mode == SImode)));
/* Determine the argument type of the builtins. The code later on
--- gcc/config/riscv/bitmanip.md.jj 2026-06-02 08:15:04.558142105 +0200
+++ gcc/config/riscv/bitmanip.md 2026-06-05 15:34:30.080083003 +0200
@@ -533,16 +533,7 @@ (define_expand "bswapdi2"
(define_expand "bswapsi2"
[(set (match_operand:SI 0 "register_operand")
(bswap:SI (match_operand:SI 1 "register_operand")))]
- "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;
-})
-
+ "(!TARGET_64BIT && (TARGET_ZBB || TARGET_ZBKB)) || TARGET_XTHEADBB")
(define_insn "*bswap<mode>2"
[(set (match_operand:X 0 "register_operand" "=r")
Jakub
