> Am 05.06.2026 um 16:00 schrieb Richard Sandiford <[email protected]>:
>
> 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 :)
LGTM
Richard
>
> 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
