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