On Thu, Oct 3, 2024 at 6:09 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> While looking into the ifcombine, I noticed that rewrite_to_defined_overflow
> was rewriting already defined code. In the previous attempt at fixing this,
> the review mentioned we should not be calling rewrite_to_defined_overflow
> in those cases. The places which called rewrite_to_defined_overflow didn't
> always check the lhs of the assignment. This fixes the problem by
> introducing a helper function which is to be used before calling
> rewrite_to_defined_overflow.
>
> Bootstrapped and tested on x86_64-linux-gnu.
OK.
Thanks,
Richard.
> gcc/ChangeLog:
>
> PR tree-optimization/111276
> * gimple-fold.cc (arith_code_with_undefined_signed_overflow): Make
> static.
> (gimple_with_undefined_signed_overflow): New function.
> * gimple-fold.h (arith_code_with_undefined_signed_overflow): Remove.
> (gimple_with_undefined_signed_overflow): Add declaration.
> * tree-if-conv.cc (if_convertible_gimple_assign_stmt_p): Use
> gimple_with_undefined_signed_overflow instead of manually
> checking lhs and the code of the stmt.
> (predicate_statements): Likewise.
> * tree-ssa-ifcombine.cc (pass_tree_ifcombine::execute): Likewise.
> * tree-ssa-loop-im.cc (move_computations_worker): Likewise.
> * tree-ssa-reassoc.cc (update_range_test): Likewise. Reformat.
> * tree-scalar-evolution.cc (final_value_replacement_loop): Use
> gimple_with_undefined_signed_overflow instead of
> arith_code_with_undefined_signed_overflow.
> * tree-ssa-loop-split.cc (split_loop): Likewise.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/gimple-fold.cc | 26 ++++++++++++++++++++++-
> gcc/gimple-fold.h | 2 +-
> gcc/tree-if-conv.cc | 16 +++------------
> gcc/tree-scalar-evolution.cc | 5 +----
> gcc/tree-ssa-ifcombine.cc | 10 ++-------
> gcc/tree-ssa-loop-im.cc | 6 +-----
> gcc/tree-ssa-loop-split.cc | 5 +----
> gcc/tree-ssa-reassoc.cc | 40 +++++++++++++++---------------------
> 8 files changed, 50 insertions(+), 60 deletions(-)
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 942de7720fd..0b49d6754e2 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -8991,7 +8991,7 @@ gimple_fold_indirect_ref (tree t)
> integer types involves undefined behavior on overflow and the
> operation can be expressed with unsigned arithmetic. */
>
> -bool
> +static bool
> arith_code_with_undefined_signed_overflow (tree_code code)
> {
> switch (code)
> @@ -9008,6 +9008,30 @@ arith_code_with_undefined_signed_overflow (tree_code
> code)
> }
> }
>
> +/* Return true if STMT has an operation that operates on a signed
> + integer types involves undefined behavior on overflow and the
> + operation can be expressed with unsigned arithmetic. */
> +
> +bool
> +gimple_with_undefined_signed_overflow (gimple *stmt)
> +{
> + if (!is_gimple_assign (stmt))
> + return false;
> + tree lhs = gimple_assign_lhs (stmt);
> + if (!lhs)
> + return false;
> + tree lhs_type = TREE_TYPE (lhs);
> + if (!INTEGRAL_TYPE_P (lhs_type)
> + && !POINTER_TYPE_P (lhs_type))
> + return false;
> + if (!TYPE_OVERFLOW_UNDEFINED (lhs_type))
> + return false;
> + if (!arith_code_with_undefined_signed_overflow
> + (gimple_assign_rhs_code (stmt)))
> + return false;
> + return true;
> +}
> +
> /* Rewrite STMT, an assignment with a signed integer or pointer arithmetic
> operation that can be transformed to unsigned arithmetic by converting
> its operand, carrying out the operation in the corresponding unsigned
> diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
> index dc709d515a9..165325392c9 100644
> --- a/gcc/gimple-fold.h
> +++ b/gcc/gimple-fold.h
> @@ -59,7 +59,7 @@ extern tree gimple_get_virt_method_for_vtable
> (HOST_WIDE_INT, tree,
> extern tree gimple_fold_indirect_ref (tree);
> extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *);
> extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *);
> -extern bool arith_code_with_undefined_signed_overflow (tree_code);
> +extern bool gimple_with_undefined_signed_overflow (gimple *);
> extern void rewrite_to_defined_overflow (gimple_stmt_iterator *);
> extern gimple_seq rewrite_to_defined_overflow (gimple *);
> extern void replace_call_with_value (gimple_stmt_iterator *, tree);
> diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
> index 3b04d1e8d34..f5aa6c04fc9 100644
> --- a/gcc/tree-if-conv.cc
> +++ b/gcc/tree-if-conv.cc
> @@ -1067,11 +1067,7 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
> fprintf (dump_file, "tree could trap...\n");
> return false;
> }
> - else if ((INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> - || POINTER_TYPE_P (TREE_TYPE (lhs)))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
> - && arith_code_with_undefined_signed_overflow
> - (gimple_assign_rhs_code (stmt)))
> + else if (gimple_with_undefined_signed_overflow (stmt))
> /* We have to rewrite stmts with undefined overflow. */
> need_to_rewrite_undefined = true;
>
> @@ -2820,7 +2816,6 @@ predicate_statements (loop_p loop)
> for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
> {
> gassign *stmt = dyn_cast <gassign *> (gsi_stmt (gsi));
> - tree lhs;
> if (!stmt)
> ;
> else if (is_false_predicate (cond)
> @@ -2876,12 +2871,7 @@ predicate_statements (loop_p loop)
>
> gsi_replace (&gsi, new_stmt, true);
> }
> - else if (((lhs = gimple_assign_lhs (stmt)), true)
> - && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> - || POINTER_TYPE_P (TREE_TYPE (lhs)))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
> - && arith_code_with_undefined_signed_overflow
> - (gimple_assign_rhs_code
> (stmt)))
> + else if (gimple_with_undefined_signed_overflow (stmt))
> rewrite_to_defined_overflow (&gsi);
> else if (gimple_vdef (stmt))
> {
> @@ -2936,7 +2926,7 @@ predicate_statements (loop_p loop)
> gsi_replace (&gsi, new_call, true);
> }
>
> - lhs = gimple_get_lhs (gsi_stmt (gsi));
> + tree lhs = gimple_get_lhs (gsi_stmt (gsi));
> if (lhs && TREE_CODE (lhs) == SSA_NAME)
> ssa_names.add (lhs);
> gsi_next (&gsi);
> diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> index abb2bad7773..34dc9810a31 100644
> --- a/gcc/tree-scalar-evolution.cc
> +++ b/gcc/tree-scalar-evolution.cc
> @@ -3931,10 +3931,7 @@ final_value_replacement_loop (class loop *loop)
> gsi2 = gsi_start (stmts);
> while (!gsi_end_p (gsi2))
> {
> - gimple *stmt = gsi_stmt (gsi2);
> - if (is_gimple_assign (stmt)
> - && arith_code_with_undefined_signed_overflow
> - (gimple_assign_rhs_code (stmt)))
> + if (gimple_with_undefined_signed_overflow (gsi_stmt (gsi2)))
> rewrite_to_defined_overflow (&gsi2);
> gsi_next (&gsi2);
> }
> diff --git a/gcc/tree-ssa-ifcombine.cc b/gcc/tree-ssa-ifcombine.cc
> index 6a3bc99190d..541a7be86ed 100644
> --- a/gcc/tree-ssa-ifcombine.cc
> +++ b/gcc/tree-ssa-ifcombine.cc
> @@ -874,15 +874,9 @@ pass_tree_ifcombine::execute (function *fun)
> for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p
> (gsi);
> gsi_next (&gsi))
> {
> - gassign *ass = dyn_cast <gassign *> (gsi_stmt (gsi));
> - if (!ass)
> + if (!gimple_with_undefined_signed_overflow (gsi_stmt (gsi)))
> continue;
> - tree lhs = gimple_assign_lhs (ass);
> - if ((INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> - || POINTER_TYPE_P (TREE_TYPE (lhs)))
> - && arith_code_with_undefined_signed_overflow
> - (gimple_assign_rhs_code (ass)))
> - rewrite_to_defined_overflow (&gsi);
> + rewrite_to_defined_overflow (&gsi);
> }
> cfg_changed |= true;
> }
> diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc
> index ccc56dc42f6..07a4739711c 100644
> --- a/gcc/tree-ssa-loop-im.cc
> +++ b/gcc/tree-ssa-loop-im.cc
> @@ -1390,11 +1390,7 @@ move_computations_worker (basic_block bb)
> when the target loop header is executed and the stmt may
> invoke undefined integer or pointer overflow rewrite it to
> unsigned arithmetic. */
> - if (is_gimple_assign (stmt)
> - && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (gimple_assign_lhs (stmt)))
> - && arith_code_with_undefined_signed_overflow
> - (gimple_assign_rhs_code (stmt))
> + if (gimple_with_undefined_signed_overflow (stmt)
> && (!ALWAYS_EXECUTED_IN (bb)
> || !(ALWAYS_EXECUTED_IN (bb) == level
> || flow_loop_nested_p (ALWAYS_EXECUTED_IN (bb), level))))
> diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
> index 68435485b79..c69403c8613 100644
> --- a/gcc/tree-ssa-loop-split.cc
> +++ b/gcc/tree-ssa-loop-split.cc
> @@ -662,10 +662,7 @@ split_loop (class loop *loop1)
> gsi = gsi_start (stmts2);
> while (!gsi_end_p (gsi))
> {
> - gimple *stmt = gsi_stmt (gsi);
> - if (is_gimple_assign (stmt)
> - && arith_code_with_undefined_signed_overflow
> - (gimple_assign_rhs_code
> (stmt)))
> + if (gimple_with_undefined_signed_overflow (gsi_stmt
> (gsi)))
> rewrite_to_defined_overflow (&gsi);
> gsi_next (&gsi);
> }
> diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
> index 347350fc98d..dc1b2410f2f 100644
> --- a/gcc/tree-ssa-reassoc.cc
> +++ b/gcc/tree-ssa-reassoc.cc
> @@ -2924,30 +2924,22 @@ update_range_test (struct range_entry *range, struct
> range_entry *otherrange,
> !gsi_end_p (gsi); gsi_next (&gsi))
> {
> gimple *stmt = gsi_stmt (gsi);
> - if (is_gimple_assign (stmt))
> - if (tree lhs = gimple_assign_lhs (stmt))
> - if ((INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> - || POINTER_TYPE_P (TREE_TYPE (lhs)))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs)))
> - {
> - enum tree_code code = gimple_assign_rhs_code (stmt);
> - if (arith_code_with_undefined_signed_overflow (code))
> - {
> - gimple_stmt_iterator gsip = gsi;
> - gimple_stmt_iterator gsin = gsi;
> - gsi_prev (&gsip);
> - gsi_next (&gsin);
> - rewrite_to_defined_overflow (&gsi);
> - unsigned uid = gimple_uid (stmt);
> - if (gsi_end_p (gsip))
> - gsip = gsi_after_labels (bb);
> - else
> - gsi_next (&gsip);
> - for (; gsi_stmt (gsip) != gsi_stmt (gsin);
> - gsi_next (&gsip))
> - gimple_set_uid (gsi_stmt (gsip), uid);
> - }
> - }
> + if (gimple_with_undefined_signed_overflow (stmt))
> + {
> + gimple_stmt_iterator gsip = gsi;
> + gimple_stmt_iterator gsin = gsi;
> + gsi_prev (&gsip);
> + gsi_next (&gsin);
> + rewrite_to_defined_overflow (&gsi);
> + unsigned uid = gimple_uid (stmt);
> + if (gsi_end_p (gsip))
> + gsip = gsi_after_labels (bb);
> + else
> + gsi_next (&gsip);
> + for (; gsi_stmt (gsip) != gsi_stmt (gsin);
> + gsi_next (&gsip))
> + gimple_set_uid (gsi_stmt (gsip), uid);
> + }
> }
>
> if (opcode == BIT_IOR_EXPR
> --
> 2.34.1
>
