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