On Fri, Jun 05, 2026 at 07:09:39PM +0100, Richard Sandiford wrote:
> Since this is kind-of my fault for pushing back against Jakub's
> original fix, how about the attached? It seems to fix the testcase
> above but I've still to check whether that's through fair means or foul.
>
> I'm not sure that the removed builtin_decl_explicit_p (BUILT_IN_BSWAP32)
> calls are necessary, since the code would go on to use a BSWAP64
> rather than a BSWAP32.
>
> Untested except for the testcase above, but I've cc:ed the patch checker.
>
> Richard
>
>
> gcc/
> * optabs-query.cc (can_open_code_p): Handle more bswap cases,
> incorporating logic from...
> * gimple-ssa-store-merging.cc (maybe_optimize_vector_constructor)
> (pass_optimize_bswap::execute)
> (imm_store_chain_info::try_coalesce_bswap): ...here. Use
> can_open_code_p instead of direct optab_handler checks.
> ---
> gcc/gimple-ssa-store-merging.cc | 30 ++++++------------------------
> gcc/optabs-query.cc | 17 +++++++++++++++++
> 2 files changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/gcc/gimple-ssa-store-merging.cc b/gcc/gimple-ssa-store-merging.cc
> index 2e5232fa65f..bd16489f30e 100644
> --- a/gcc/gimple-ssa-store-merging.cc
> +++ b/gcc/gimple-ssa-store-merging.cc
> @@ -1570,10 +1570,7 @@ maybe_optimize_vector_constructor (gimple *cur_stmt)
> break;
> case 32:
> if (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
> - && (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))
> + && can_implement_p (bswap_optab, SImode))
I think we really want can_open_code_p in all the
gimple-ssa-store-merging.cc spots. Libcall isn't going to be a good idea
compared to just multiple loads/stores IMHO.
> --- a/gcc/optabs-query.cc
> +++ b/gcc/optabs-query.cc
> @@ -840,6 +840,23 @@ can_open_code_p (optab op, machine_mode mode)
> && can_implement_p (op == neg_optab ? xor_optab : and_optab, new_mode))
Not related to this patch, but I wonder if the above shouldn't actually be
can_open_code_p rather than can_implement_p...
> return true;
>
> + scalar_int_mode int_mode;
> + if (op == bswap_optab && is_a<scalar_int_mode> (mode, &int_mode))
Why not (op == bswap_optab || op == bitreverse_optab) when we are at it?
> + {
> + /* widen_bswap_or_bitreverse can implement smaller bswaps using
> + wider bswaps and a shift. */
> + opt_scalar_int_mode wider_mode_iter;
> + FOR_EACH_WIDER_MODE (wider_mode_iter, int_mode)
> + if (optab_handler (op, wider_mode_iter.require ()) != CODE_FOR_nothing)
> + return true;
> +
> + /* expand_doubleword_bswap_or_bitreverse can use 2 word bswaps to
> + implement a doubleword bswap. */
> + if (GET_MODE_SIZE (int_mode) == 2 * UNITS_PER_WORD
> + && optab_handler (op, word_mode) != CODE_FOR_nothing)
> + return true;
> + }
> +
> return false;
> }
Jakub
