On Sun, Oct 5, 2025 at 12:01 AM Andrew Pinski <[email protected]> wrote: > > This is a small compile time optimization where if commonalizing stores > that have the same rhs, a phi node does not need to be created. > This uses the same code as what was added for the `= {};` case. > The reason why it is a compile time optimization is that Copy prop > later on will do the same thing so not creating a new phi and a new > ssa name will have a small compile time improvement.
OK. I'll note that both this and sinking will eventually create PHIs that can be CSEd. Not sure whether it's worth implementing a simple CSE for this here (also considering CSE to already existing PHIs) RIchard. > Bootstrapped and tested on x86_64-linux-gnu. > > PR tree-optimization/122155 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (cond_if_else_store_replacement_1): Don't > create a phi if the 2 rhs are the same. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/cselim-3.c: New test. > > Signed-off-by: Andrew Pinski <[email protected]> > --- > gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c | 16 ++++++++++++++++ > gcc/tree-ssa-phiopt.cc | 10 ++++------ > 2 files changed, 20 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c > b/gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c > new file mode 100644 > index 00000000000..d0c8e6a9e18 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-phiopt1-details-stats" } */ > +/* PR tree-optimization/122155 */ > + > +void f(int *a, int b, int c) > +{ > + if (c) > + *a = b; > + else > + *a = b; > +} > + > +/* When commonalizing a store with the same rhs, a PHI does not need to be > created. */ > +/* { dg-final { scan-tree-dump "if-then-else store replacement: 1" "phiopt1" > } } */ > +/* { dg-final { scan-tree-dump-not "to use phi:" "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-not "PHI <" "phiopt1" } } */ > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index 5d0c867f99a..094828ff9f1 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -3643,9 +3643,8 @@ cond_if_else_store_replacement_1 (basic_block then_bb, > basic_block else_bb, > tree lhs_base, lhs, then_rhs, else_rhs, name; > location_t then_locus, else_locus; > gimple_stmt_iterator gsi; > - gphi *newphi; > + gphi *newphi = nullptr; > gassign *new_stmt; > - bool empty_constructor = false; > > if (then_assign == NULL > || !gimple_assign_single_p (then_assign) > @@ -3680,7 +3679,6 @@ cond_if_else_store_replacement_1 (basic_block then_bb, > basic_block else_bb, > /* Currently only handle commoning of `= {}`. */ > if (TREE_CODE (then_rhs) != CONSTRUCTOR) > return false; > - empty_constructor = true; > } > > if (dump_file && (dump_flags & TDF_DETAILS)) > @@ -3709,8 +3707,8 @@ cond_if_else_store_replacement_1 (basic_block then_bb, > basic_block else_bb, > /* 2) Create a PHI node at the join block, with one argument > holding the old RHS, and the other holding the temporary > where we stored the old memory contents. */ > - if (empty_constructor) > - name = unshare_expr (then_rhs); > + if (operand_equal_p (then_rhs, else_rhs)) > + name = then_rhs; > else > { > name = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "cstore"); > @@ -3730,7 +3728,7 @@ cond_if_else_store_replacement_1 (basic_block then_bb, > basic_block else_bb, > update_stmt (vphi); > if (dump_file && (dump_flags & TDF_DETAILS)) > { > - if (!empty_constructor) > + if (newphi) > { > fprintf(dump_file, "to use phi:\n"); > print_gimple_stmt (dump_file, newphi, 0, > -- > 2.43.0 >
