On Mon, Oct 6, 2025 at 10:37 AM Andrew Pinski <[email protected]> wrote: > > On Sun, Oct 5, 2025 at 11:02 PM Richard Biener > <[email protected]> wrote: > > > > On Sat, Oct 4, 2025 at 2:43 AM Andrew Pinski > > <[email protected]> wrote: > > > > > > Currently cselim and cselim-limited are able to > > > handle stores which have a rhs of a ssa name or a constant. > > > This extends that support to also allow `= {}`. > > > The sink pass will also commonalize the store but in some > > > cases this is too late in the pipeline. Doing it in phiopt1 > > > allows for better inlining estimates too. > > > > > > This is also the first step in improving/fixing PR 122083 > > > such that we do an early inlining which is now not happening > > > for GCC 15+. > > > > ISTR sinking also handles commoning of clobbers but > > operand_equal_p will reject those. > > Yes it might be a good idea to commoning of clobbers though getting a > testcase there is harder; requires jump threading to duplicate a bb . > Also there is a check for clobbers earlier in > cond_if_else_store_replacement_1 before the newly added > operand_equal_p. > > I will try to find one and file a bug report about it.
Found one without the need of jump threading. C++ front-end adding a clobber after inplacement new makes it easier to produce a testcase in the end. Filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122178 . Thanks, Andrew > > Thanks, > Andrew > > > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > > > PR tree-optimization/122153 > > > > > > gcc/ChangeLog: > > > > > > * tree-ssa-phiopt.cc (cond_if_else_store_replacement_1): Handle > > > stores of empty constructors too. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.dg/tree-ssa/pr122153-1.c: New test. > > > > > > Signed-off-by: Andrew Pinski <[email protected]> > > > --- > > > gcc/testsuite/gcc.dg/tree-ssa/pr122153-1.c | 30 +++++++++++++++ > > > gcc/tree-ssa-phiopt.cc | 44 ++++++++++++++++------ > > > 2 files changed, 62 insertions(+), 12 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr122153-1.c > > > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr122153-1.c > > > b/gcc/testsuite/gcc.dg/tree-ssa/pr122153-1.c > > > new file mode 100644 > > > index 00000000000..d6e7527aaa6 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr122153-1.c > > > @@ -0,0 +1,30 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-O2 -fdump-tree-phiopt1" } */ > > > +/* PR tree-optimization/122153 */ > > > +struct s1 > > > +{ > > > + int t; > > > +}; > > > +void g(struct s1*); > > > +struct s1 f(int *a, int c, int d) > > > +{ > > > + struct s1 r; > > > + int t1; > > > + if (c < d) > > > + { > > > + r = (struct s1){}; > > > + t1 = c; > > > + } > > > + else > > > + { > > > + r = (struct s1){}; > > > + t1 = d; > > > + } > > > + g(&r); > > > + r.t = t1; > > > + return r; > > > +} > > > +/* the `r = {};` store should be commonialized out of the conditional > > > + and produce a MIN_EXPR in phiopt1. */ > > > + > > > +/* { dg-final { scan-tree-dump "MIN_EXPR" "phiopt1" } } */ > > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > > > index e1c9e1296c0..bdd1f7396da 100644 > > > --- a/gcc/tree-ssa-phiopt.cc > > > +++ b/gcc/tree-ssa-phiopt.cc > > > @@ -3645,6 +3645,7 @@ cond_if_else_store_replacement_1 (basic_block > > > then_bb, basic_block else_bb, > > > gimple_stmt_iterator gsi; > > > gphi *newphi; > > > gassign *new_stmt; > > > + bool empty_constructor = false; > > > > > > if (then_assign == NULL > > > || !gimple_assign_single_p (then_assign) > > > @@ -3659,8 +3660,7 @@ cond_if_else_store_replacement_1 (basic_block > > > then_bb, basic_block else_bb, > > > return false; > > > > > > lhs = gimple_assign_lhs (then_assign); > > > - if (!is_gimple_reg_type (TREE_TYPE (lhs)) > > > - || !operand_equal_p (lhs, gimple_assign_lhs (else_assign), 0)) > > > + if (!operand_equal_p (lhs, gimple_assign_lhs (else_assign), 0)) > > > return false; > > > > > > lhs_base = get_base_address (lhs); > > > @@ -3673,6 +3673,16 @@ cond_if_else_store_replacement_1 (basic_block > > > then_bb, basic_block else_bb, > > > then_locus = gimple_location (then_assign); > > > else_locus = gimple_location (else_assign); > > > > > > + if (!is_gimple_reg_type (TREE_TYPE (lhs))) > > > + { > > > + if (!operand_equal_p (then_rhs, else_rhs)) > > > + return false; > > > + /* Currently only handle commoning of `= {}`. */ > > > + if (TREE_CODE (then_rhs) != CONSTRUCTOR) > > > + return false; > > > + empty_constructor = true; > > > + } > > > + > > > if (dump_file && (dump_flags & TDF_DETAILS)) > > > { > > > fprintf(dump_file, "factoring out stores:\n\tthen:\n"); > > > @@ -3699,12 +3709,17 @@ 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. */ > > > - name = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "cstore"); > > > - newphi = create_phi_node (name, join_bb); > > > - add_phi_arg (newphi, then_rhs, EDGE_SUCC (then_bb, 0), then_locus); > > > - add_phi_arg (newphi, else_rhs, EDGE_SUCC (else_bb, 0), else_locus); > > > + if (empty_constructor) > > > + name = unshare_expr (then_rhs); > > > + else > > > + { > > > + name = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "cstore"); > > > + newphi = create_phi_node (name, join_bb); > > > + add_phi_arg (newphi, then_rhs, EDGE_SUCC (then_bb, 0), then_locus); > > > + add_phi_arg (newphi, else_rhs, EDGE_SUCC (else_bb, 0), else_locus); > > > + } > > > > > > - new_stmt = gimple_build_assign (lhs, gimple_phi_result (newphi)); > > > + new_stmt = gimple_build_assign (lhs, name); > > > /* Update the vdef for the new store statement. */ > > > tree newvphilhs = make_ssa_name (gimple_vop (cfun)); > > > tree vdef = gimple_phi_result (vphi); > > > @@ -3715,16 +3730,21 @@ cond_if_else_store_replacement_1 (basic_block > > > then_bb, basic_block else_bb, > > > update_stmt (vphi); > > > if (dump_file && (dump_flags & TDF_DETAILS)) > > > { > > > - fprintf(dump_file, "to use phi:\n"); > > > - print_gimple_stmt (dump_file, newphi, 0, > > > - TDF_VOPS|TDF_MEMSYMS); > > > - fprintf(dump_file, "\n"); > > > + if (!empty_constructor) > > > + { > > > + fprintf(dump_file, "to use phi:\n"); > > > + print_gimple_stmt (dump_file, newphi, 0, > > > + TDF_VOPS|TDF_MEMSYMS); > > > + fprintf(dump_file, "\n"); > > > + } > > > + else > > > + fprintf(dump_file, "to:\n"); > > > print_gimple_stmt (dump_file, new_stmt, 0, > > > TDF_VOPS|TDF_MEMSYMS); > > > fprintf(dump_file, "\n\n"); > > > } > > > > > > - /* 3) Insert that PHI node. */ > > > + /* 3) Insert that new store. */ > > > gsi = gsi_after_labels (join_bb); > > > if (gsi_end_p (gsi)) > > > { > > > -- > > > 2.43.0 > > >
