On Fri, Oct 4, 2024 at 12:08 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> 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.
Just a quick note, this depends on the rewrite of `scope conflicts
better` issue; otherwise we get a regression. I am working on the new
patch for that and will resubmit this with that patch.
Thanks,
Andrew
>
> 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
> >
