Jakub Jelinek <[email protected]> writes:
> 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.
Oops, yes. As per the change-log, that's what I'd intended. Guess in
the rush I must have copy-&-pasted the wrong name :(
Sorry for the mix-up.
>> --- 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...
Yeah, probably, but I'll look at it when it isn't Friday evening.
>
>> 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?
I'd wondered about that, but in the circumstances, it seemed better
not to start adding things that wouldn't be tested.
I can add it if you prefer though.
Anyway, here's the patch again with the right function name.
I've changed the subject line because I think the patch checker
might have ignored the last version.
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..71cb7d8031a 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_open_code_p (bswap_optab, SImode))
{
load_type = uint32_type_node;
fndecl = builtin_decl_explicit (BUILT_IN_BSWAP32);
@@ -1584,12 +1581,7 @@ maybe_optimize_vector_constructor (gimple *cur_stmt)
break;
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)))
+ && can_open_code_p (bswap_optab, DImode))
{
load_type = uint64_type_node;
fndecl = builtin_decl_explicit (BUILT_IN_BSWAP64);
@@ -1636,16 +1628,9 @@ pass_optimize_bswap::execute (function *fun)
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
- /* widen_bswap_or_bitreverse can implement 32-bit bswap
- using bswapdi2 and shift. */
- || (optab_handler (bswap_optab, DImode)
- != CODE_FOR_nothing)));
+ && can_open_code_p (bswap_optab, SImode));
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)));
+ && can_open_code_p (bswap_optab, DImode));
/* Determine the argument type of the builtins. The code later on
assumes that the return and argument type are the same. */
@@ -3145,15 +3130,12 @@ imm_store_chain_info::try_coalesce_bswap
(merged_store_group *merged_store,
break;
case 32:
if (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
- && optab_handler (bswap_optab, SImode) != CODE_FOR_nothing)
+ && can_open_code_p (bswap_optab, SImode))
break;
return false;
case 64:
if (builtin_decl_explicit_p (BUILT_IN_BSWAP64)
- && (optab_handler (bswap_optab, DImode) != CODE_FOR_nothing
- || (word_mode == SImode
- && builtin_decl_explicit_p (BUILT_IN_BSWAP32)
- && optab_handler (bswap_optab, SImode) !=
CODE_FOR_nothing)))
+ && can_open_code_p (bswap_optab, DImode))
break;
return false;
default:
diff --git a/gcc/optabs-query.cc b/gcc/optabs-query.cc
index e596d8db881..8226356c7d4 100644
--- 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))
return true;
+ scalar_int_mode int_mode;
+ if (op == bswap_optab && is_a<scalar_int_mode> (mode, &int_mode))
+ {
+ /* 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;
}
--
2.54.0
