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.
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
