Re: [PATCH] store-merging: Handle vector CONSTRUCTORs using bswap [PR96239]
On Tue, Jan 05, 2021 at 01:55:21PM +0100, Richard Biener wrote:
> > Note, I have no idea why the bswap code needs TODO_update_ssa if it changed
> > things, for the vuses it copies them from the surrounding vuses, which looks
> > correct to me. Perhaps because it uses force_gimple_operand_gsi* in a few
> > spots in bswap_replace? Confused...
>
> .. that shouldn't cause updating SSA to be necessary. Maybe it at some
> point did not update virtual operands appropriately.
Ok, I've committed the following version without the TODO_update_ssa, which
passed another bootstrap/regtest on x86_64-linux and i686-linux.
2021-01-05 Jakub Jelinek
PR tree-optimization/96239
* gimple-ssa-store-merging.c (maybe_optimize_vector_constructor): New
function.
(get_status_for_store_merging): Don't return BB_INVALID for blocks
with potential bswap optimizable CONSTRUCTORs.
(pass_store_merging::execute): Optimize vector CONSTRUCTORs with bswap
if possible.
* gcc.dg/tree-ssa/pr96239.c: New test.
--- gcc/gimple-ssa-store-merging.c.jj 2020-12-16 13:07:51.729733816 +0100
+++ gcc/gimple-ssa-store-merging.c 2020-12-16 16:02:06.238868137 +0100
@@ -1255,6 +1255,75 @@ bswap_replace (gimple_stmt_iterator gsi,
return tgt;
}
+/* Try to optimize an assignment CUR_STMT with CONSTRUCTOR on the rhs
+ using bswap optimizations. CDI_DOMINATORS need to be
+ computed on entry. Return true if it has been optimized and
+ TODO_update_ssa is needed. */
+
+static bool
+maybe_optimize_vector_constructor (gimple *cur_stmt)
+{
+ tree fndecl = NULL_TREE, bswap_type = NULL_TREE, load_type;
+ struct symbolic_number n;
+ bool bswap;
+
+ gcc_assert (is_gimple_assign (cur_stmt)
+ && gimple_assign_rhs_code (cur_stmt) == CONSTRUCTOR);
+
+ tree rhs = gimple_assign_rhs1 (cur_stmt);
+ if (!VECTOR_TYPE_P (TREE_TYPE (rhs))
+ || !INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (rhs)))
+ || gimple_assign_lhs (cur_stmt) == NULL_TREE)
+return false;
+
+ HOST_WIDE_INT sz = int_size_in_bytes (TREE_TYPE (rhs)) * BITS_PER_UNIT;
+ switch (sz)
+{
+case 16:
+ load_type = bswap_type = uint16_type_node;
+ break;
+case 32:
+ if (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
+ && optab_handler (bswap_optab, SImode) != CODE_FOR_nothing)
+ {
+ load_type = uint32_type_node;
+ fndecl = builtin_decl_explicit (BUILT_IN_BSWAP32);
+ bswap_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+ }
+ else
+ return false;
+ break;
+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)))
+ {
+ load_type = uint64_type_node;
+ fndecl = builtin_decl_explicit (BUILT_IN_BSWAP64);
+ bswap_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+ }
+ else
+ return false;
+ break;
+default:
+ return false;
+}
+
+ gimple *ins_stmt = find_bswap_or_nop (cur_stmt, , );
+ if (!ins_stmt || n.range != (unsigned HOST_WIDE_INT) sz)
+return false;
+
+ if (bswap && !fndecl && n.range != 16)
+return false;
+
+ memset (_stats, 0, sizeof (nop_stats));
+ memset (_stats, 0, sizeof (bswap_stats));
+ return bswap_replace (gsi_for_stmt (cur_stmt), ins_stmt, fndecl,
+ bswap_type, load_type, , bswap) != NULL_TREE;
+}
+
/* Find manual byte swap implementations as well as load in a given
endianness. Byte swaps are turned into a bswap builtin invokation
while endian loads are converted to bswap builtin invokation or
@@ -5126,6 +5195,7 @@ static enum basic_block_status
get_status_for_store_merging (basic_block bb)
{
unsigned int num_statements = 0;
+ unsigned int num_constructors = 0;
gimple_stmt_iterator gsi;
edge e;
@@ -5138,9 +5208,27 @@ get_status_for_store_merging (basic_bloc
if (store_valid_for_store_merging_p (stmt) && ++num_statements >= 2)
break;
+
+ if (is_gimple_assign (stmt)
+ && gimple_assign_rhs_code (stmt) == CONSTRUCTOR)
+ {
+ tree rhs = gimple_assign_rhs1 (stmt);
+ if (VECTOR_TYPE_P (TREE_TYPE (rhs))
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (rhs)))
+ && gimple_assign_lhs (stmt) != NULL_TREE)
+ {
+ HOST_WIDE_INT sz
+ = int_size_in_bytes (TREE_TYPE (rhs)) * BITS_PER_UNIT;
+ if (sz == 16 || sz == 32 || sz == 64)
+ {
+ num_constructors = 1;
+ break;
+ }
+ }
+ }
}
- if (num_statements == 0)
+ if (num_statements == 0 && num_constructors == 0)
return BB_INVALID;
if (cfun->can_throw_non_call_exceptions && cfun->eh
@@
Re: [PATCH] store-merging: Handle vector CONSTRUCTORs using bswap [PR96239]
On Thu, 17 Dec 2020, Jakub Jelinek wrote:
> On Wed, Dec 16, 2020 at 09:29:31AM +0100, Richard Biener wrote:
> > I think it probably makes sense to have some helper split out that
> > collects & classifies vector constructor components we can use from
> > both forwprop (where matching the V_C_E from integer could be done
> > as well IMHO) and bswap (when a permute is involved) and store-merging.
>
> I've tried to add such helper, but handling over just analysis and letting
> each pass handle it differently seems complicated given the limitations of
> the bswap infrastructure.
>
> So, this patch just hooks the optimization also into store-merging so that
> the original testcase from the PR can be fixed.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK, but ...
> Note, I have no idea why the bswap code needs TODO_update_ssa if it changed
> things, for the vuses it copies them from the surrounding vuses, which looks
> correct to me. Perhaps because it uses force_gimple_operand_gsi* in a few
> spots in bswap_replace? Confused...
.. that shouldn't cause updating SSA to be necessary. Maybe it at some
point did not update virtual operands appropriately.
Richard.
> 2020-12-17 Jakub Jelinek
>
> PR tree-optimization/96239
> * gimple-ssa-store-merging.c (maybe_optimize_vector_constructor): New
> function.
> (get_status_for_store_merging): Don't return BB_INVALID for blocks
> with potential bswap optimizable CONSTRUCTORs.
> (pass_store_merging::execute): Optimize vector CONSTRUCTORs with bswap
> if possible.
>
> * gcc.dg/tree-ssa/pr96239.c: New test.
>
> --- gcc/gimple-ssa-store-merging.c.jj 2020-12-16 13:07:51.729733816 +0100
> +++ gcc/gimple-ssa-store-merging.c2020-12-16 16:02:06.238868137 +0100
> @@ -1255,6 +1255,75 @@ bswap_replace (gimple_stmt_iterator gsi,
>return tgt;
> }
>
> +/* Try to optimize an assignment CUR_STMT with CONSTRUCTOR on the rhs
> + using bswap optimizations. CDI_DOMINATORS need to be
> + computed on entry. Return true if it has been optimized and
> + TODO_update_ssa is needed. */
> +
> +static bool
> +maybe_optimize_vector_constructor (gimple *cur_stmt)
> +{
> + tree fndecl = NULL_TREE, bswap_type = NULL_TREE, load_type;
> + struct symbolic_number n;
> + bool bswap;
> +
> + gcc_assert (is_gimple_assign (cur_stmt)
> + && gimple_assign_rhs_code (cur_stmt) == CONSTRUCTOR);
> +
> + tree rhs = gimple_assign_rhs1 (cur_stmt);
> + if (!VECTOR_TYPE_P (TREE_TYPE (rhs))
> + || !INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (rhs)))
> + || gimple_assign_lhs (cur_stmt) == NULL_TREE)
> +return false;
> +
> + HOST_WIDE_INT sz = int_size_in_bytes (TREE_TYPE (rhs)) * BITS_PER_UNIT;
> + switch (sz)
> +{
> +case 16:
> + load_type = bswap_type = uint16_type_node;
> + break;
> +case 32:
> + if (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
> + && optab_handler (bswap_optab, SImode) != CODE_FOR_nothing)
> + {
> + load_type = uint32_type_node;
> + fndecl = builtin_decl_explicit (BUILT_IN_BSWAP32);
> + bswap_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
> + }
> + else
> + return false;
> + break;
> +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)))
> + {
> + load_type = uint64_type_node;
> + fndecl = builtin_decl_explicit (BUILT_IN_BSWAP64);
> + bswap_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
> + }
> + else
> + return false;
> + break;
> +default:
> + return false;
> +}
> +
> + gimple *ins_stmt = find_bswap_or_nop (cur_stmt, , );
> + if (!ins_stmt || n.range != (unsigned HOST_WIDE_INT) sz)
> +return false;
> +
> + if (bswap && !fndecl && n.range != 16)
> +return false;
> +
> + memset (_stats, 0, sizeof (nop_stats));
> + memset (_stats, 0, sizeof (bswap_stats));
> + return bswap_replace (gsi_for_stmt (cur_stmt), ins_stmt, fndecl,
> + bswap_type, load_type, , bswap) != NULL_TREE;
> +}
> +
> /* Find manual byte swap implementations as well as load in a given
> endianness. Byte swaps are turned into a bswap builtin invokation
> while endian loads are converted to bswap builtin invokation or
> @@ -5126,6 +5195,7 @@ static enum basic_block_status
> get_status_for_store_merging (basic_block bb)
> {
>unsigned int num_statements = 0;
> + unsigned int num_constructors = 0;
>gimple_stmt_iterator gsi;
>edge e;
>
> @@ -5138,9 +5208,27 @@ get_status_for_store_merging (basic_bloc
>
>if (store_valid_for_store_merging_p (stmt) && ++num_statements >= 2)
> break;
> +
> + if (is_gimple_assign (stmt)
> + &&
[PATCH] store-merging: Handle vector CONSTRUCTORs using bswap [PR96239]
On Wed, Dec 16, 2020 at 09:29:31AM +0100, Richard Biener wrote:
> I think it probably makes sense to have some helper split out that
> collects & classifies vector constructor components we can use from
> both forwprop (where matching the V_C_E from integer could be done
> as well IMHO) and bswap (when a permute is involved) and store-merging.
I've tried to add such helper, but handling over just analysis and letting
each pass handle it differently seems complicated given the limitations of
the bswap infrastructure.
So, this patch just hooks the optimization also into store-merging so that
the original testcase from the PR can be fixed.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Note, I have no idea why the bswap code needs TODO_update_ssa if it changed
things, for the vuses it copies them from the surrounding vuses, which looks
correct to me. Perhaps because it uses force_gimple_operand_gsi* in a few
spots in bswap_replace? Confused...
2020-12-17 Jakub Jelinek
PR tree-optimization/96239
* gimple-ssa-store-merging.c (maybe_optimize_vector_constructor): New
function.
(get_status_for_store_merging): Don't return BB_INVALID for blocks
with potential bswap optimizable CONSTRUCTORs.
(pass_store_merging::execute): Optimize vector CONSTRUCTORs with bswap
if possible.
* gcc.dg/tree-ssa/pr96239.c: New test.
--- gcc/gimple-ssa-store-merging.c.jj 2020-12-16 13:07:51.729733816 +0100
+++ gcc/gimple-ssa-store-merging.c 2020-12-16 16:02:06.238868137 +0100
@@ -1255,6 +1255,75 @@ bswap_replace (gimple_stmt_iterator gsi,
return tgt;
}
+/* Try to optimize an assignment CUR_STMT with CONSTRUCTOR on the rhs
+ using bswap optimizations. CDI_DOMINATORS need to be
+ computed on entry. Return true if it has been optimized and
+ TODO_update_ssa is needed. */
+
+static bool
+maybe_optimize_vector_constructor (gimple *cur_stmt)
+{
+ tree fndecl = NULL_TREE, bswap_type = NULL_TREE, load_type;
+ struct symbolic_number n;
+ bool bswap;
+
+ gcc_assert (is_gimple_assign (cur_stmt)
+ && gimple_assign_rhs_code (cur_stmt) == CONSTRUCTOR);
+
+ tree rhs = gimple_assign_rhs1 (cur_stmt);
+ if (!VECTOR_TYPE_P (TREE_TYPE (rhs))
+ || !INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (rhs)))
+ || gimple_assign_lhs (cur_stmt) == NULL_TREE)
+return false;
+
+ HOST_WIDE_INT sz = int_size_in_bytes (TREE_TYPE (rhs)) * BITS_PER_UNIT;
+ switch (sz)
+{
+case 16:
+ load_type = bswap_type = uint16_type_node;
+ break;
+case 32:
+ if (builtin_decl_explicit_p (BUILT_IN_BSWAP32)
+ && optab_handler (bswap_optab, SImode) != CODE_FOR_nothing)
+ {
+ load_type = uint32_type_node;
+ fndecl = builtin_decl_explicit (BUILT_IN_BSWAP32);
+ bswap_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+ }
+ else
+ return false;
+ break;
+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)))
+ {
+ load_type = uint64_type_node;
+ fndecl = builtin_decl_explicit (BUILT_IN_BSWAP64);
+ bswap_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+ }
+ else
+ return false;
+ break;
+default:
+ return false;
+}
+
+ gimple *ins_stmt = find_bswap_or_nop (cur_stmt, , );
+ if (!ins_stmt || n.range != (unsigned HOST_WIDE_INT) sz)
+return false;
+
+ if (bswap && !fndecl && n.range != 16)
+return false;
+
+ memset (_stats, 0, sizeof (nop_stats));
+ memset (_stats, 0, sizeof (bswap_stats));
+ return bswap_replace (gsi_for_stmt (cur_stmt), ins_stmt, fndecl,
+ bswap_type, load_type, , bswap) != NULL_TREE;
+}
+
/* Find manual byte swap implementations as well as load in a given
endianness. Byte swaps are turned into a bswap builtin invokation
while endian loads are converted to bswap builtin invokation or
@@ -5126,6 +5195,7 @@ static enum basic_block_status
get_status_for_store_merging (basic_block bb)
{
unsigned int num_statements = 0;
+ unsigned int num_constructors = 0;
gimple_stmt_iterator gsi;
edge e;
@@ -5138,9 +5208,27 @@ get_status_for_store_merging (basic_bloc
if (store_valid_for_store_merging_p (stmt) && ++num_statements >= 2)
break;
+
+ if (is_gimple_assign (stmt)
+ && gimple_assign_rhs_code (stmt) == CONSTRUCTOR)
+ {
+ tree rhs = gimple_assign_rhs1 (stmt);
+ if (VECTOR_TYPE_P (TREE_TYPE (rhs))
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (rhs)))
+ && gimple_assign_lhs (stmt) != NULL_TREE)
+ {
+ HOST_WIDE_INT sz
+ = int_size_in_bytes (TREE_TYPE (rhs)) *
