On Thu, 13 Feb 2025, Richard Biener wrote:
> 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.
Ah, but it can be a constant. I'm sending out a v2 using
gimple_build instead so if it's a constant it will be folded.
Richard.
> 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)
