On Sun, Oct 5, 2025 at 11:12 PM Richard Biener <[email protected]> wrote: > > 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)
Yes It might be useful to do one. I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122181 for a simple example. I have some ideas on how to implement it but I also want to see how often it shows up in some C++ code before deciding if it is a good idea to implement or not. Thanks, Andrew > > 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 > >
