On Wed, 12 Feb 2025, Andrew Pinski wrote:
> On Wed, Feb 12, 2025 at 4:04 AM Richard Biener <[email protected]> wrote:
> >
> > The PR indicates a very specific issue with regard to SSA coalescing
> > failures because there's a pre IV increment loop exit test. While
> > IVOPTs created the desired IL we later simplify the exit test into
> > the undesirable form again. The following fixes this up during RTL
> > expansion where we try to improve coalescing of IVs. That seems
> > easier that trying to avoid the simplification with some weird
> > heuristics (it could also have been written this way).
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > OK for trunk?
> >
> > Thanks,
> > Richard.
> >
> > PR tree-optimization/86270
> > * tree-outof-ssa.cc (insert_backedge_copies): Pattern
> > match a single conflict in a loop condition and adjust
> > that avoiding the conflict if possible.
> >
> > * gcc.target/i386/pr86270.c: Adjust to check for no reg-reg
> > copies as well.
> > ---
> > gcc/testsuite/gcc.target/i386/pr86270.c | 3 ++
> > gcc/tree-outof-ssa.cc | 49 ++++++++++++++++++++++---
> > 2 files changed, 47 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/pr86270.c
> > b/gcc/testsuite/gcc.target/i386/pr86270.c
> > index 68562446fa4..89b9aeb317a 100644
> > --- a/gcc/testsuite/gcc.target/i386/pr86270.c
> > +++ b/gcc/testsuite/gcc.target/i386/pr86270.c
> > @@ -13,3 +13,6 @@ test ()
> >
> > /* Check we do not split the backedge but keep nice loop form. */
> > /* { dg-final { scan-assembler-times "L\[0-9\]+:" 2 } } */
> > +/* Check we do not end up with reg-reg moves from a pre-increment IV
> > + exit test. */
> > +/* { dg-final { scan-assembler-not "mov\[lq\]\?\t%\?\[er\].x, %\?\[er\].x"
> > } } */
> > diff --git a/gcc/tree-outof-ssa.cc b/gcc/tree-outof-ssa.cc
> > index d340d4ba529..f285c81599e 100644
> > --- a/gcc/tree-outof-ssa.cc
> > +++ b/gcc/tree-outof-ssa.cc
> > @@ -1259,10 +1259,9 @@ insert_backedge_copies (void)
> > if (gimple_nop_p (def)
> > || gimple_code (def) == GIMPLE_PHI)
> > continue;
> > - tree name = copy_ssa_name (result);
> > - gimple *stmt = gimple_build_assign (name, result);
> > imm_use_iterator imm_iter;
> > gimple *use_stmt;
> > + auto_vec<use_operand_p, 8> uses;
> > /* The following matches trivially_conflicts_p. */
> > FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, result)
> > {
> > @@ -1273,11 +1272,51 @@ insert_backedge_copies (void)
> > {
> > use_operand_p use;
> > FOR_EACH_IMM_USE_ON_STMT (use, imm_iter)
> > - SET_USE (use, name);
> > + uses.safe_push (use);
> > }
> > }
> > - gimple_stmt_iterator gsi = gsi_for_stmt (def);
> > - gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
> > + /* When there is just a conflicting statement try to
> > + adjust that to refer to the new definition.
> > + In particular for now handle a conflict with the
> > + use in a (exit) condition with a NE compare,
> > + replacing a pre-IV-increment compare with a
> > + post-IV-increment one. */
> > + if (uses.length () == 1
> > + && is_a <gcond *> (USE_STMT (uses[0]))
> > + && gimple_cond_code (USE_STMT (uses[0])) == NE_EXPR
> > + && is_gimple_assign (def)
> > + && gimple_assign_rhs1 (def) == result
> > + && (gimple_assign_rhs_code (def) == PLUS_EXPR
> > + || gimple_assign_rhs_code (def) == MINUS_EXPR
> > + || gimple_assign_rhs_code (def) ==
> > POINTER_PLUS_EXPR)
> > + && TREE_CODE (gimple_assign_rhs2 (def)) ==
> > INTEGER_CST)
> > + {
> > + gcond *cond = as_a <gcond *> (USE_STMT (uses[0]));
> > + tree *adj;
> > + if (gimple_cond_lhs (cond) == result)
> > + adj = gimple_cond_rhs_ptr (cond);
> > + else
> > + adj = gimple_cond_lhs_ptr (cond);
> > + tree name = copy_ssa_name (result);
>
> Should this be `copy_ssa_name (*adj)`? Since the new name is based on
> `*adj` rather than based on the result.
Good point, I've adjusted this in my local copy.
Richard.
> Thanks,
> Andrew Pinski
>
> > + gimple *stmt
> > + = gimple_build_assign (name,
> > + gimple_assign_rhs_code (def),
> > + *adj, gimple_assign_rhs2
> > (def));
> > + gimple_stmt_iterator gsi = gsi_for_stmt (cond);
> > + gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
> > + *adj = name;
> > + SET_USE (uses[0], arg);
> > + update_stmt (cond);
> > + }
> > + else
> > + {
> > + tree name = copy_ssa_name (result);
> > + gimple *stmt = gimple_build_assign (name, result);
> > + gimple_stmt_iterator gsi = gsi_for_stmt (def);
> > + gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
> > + for (auto use : uses)
> > + SET_USE (use, name);
> > + }
> > }
> > }
> > }
> > --
> > 2.43.0
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
