Jakub Jelinek <[email protected]> writes: > 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.
FWIW you can cc [email protected] to get precommit testing of RISC-V testing, with the results showing up at https://github.com/ewlu/gcc-precommit-ci . > 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. Thanks for doing this. The patch LGTM, but it's not exacty my area, so Richi should have the final say :) Richard > > --- 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
