On Thu, Oct 30, 2025 at 1:34 AM Richard Biener <[email protected]> wrote: > > On Wed, Oct 29, 2025 at 5:20 PM Andrew Pinski > <[email protected]> wrote: > > > > In this case we have a phi node for the use so we need to see if > > the result of the phi is a single usage with the clobber. > > > > That is the following IR: > > ``` > > # .MEM_6 = VDEF <.MEM_5(D)> > > inner = outer; > > # .MEM_7 = VDEF <.MEM_6> > > p (outer); > > > > <bb 3> : > > ... > > # .MEM_8 = VDEF <.MEM_7> > > g (_3, _2, _1); > > > > <bb 4> : > > # .MEM_9 = VDEF <.MEM_8> > > inner ={v} {CLOBBER(eos)}; > > ... > > > > <bb 5> : > > # .MEM_4 = PHI <.MEM_7(2), .MEM_8(3)> > > <L0>: > > # .MEM_10 = VDEF <.MEM_4> > > inner ={v} {CLOBBER(eos)}; > > ``` > > > > The two two clobber can be considered the same. > > So starting at `bb 4`'s. Bofore we walk back to the call of g statement > > and would notice that the use in the phi node of `bb5` and that would cause > > the walk to stop. But in this case since he phi node has a single use of the > > clobber and the clobber matches the original clobber it can be considered > > the > > same "one". So with the patch now, we walk back one more statement and > > allow it. > > Similar to the at the call to p statement. > > > > Bootstrapped and tested on x86_64-linux-gnu. > > OK. > > I'll note this "simple" DSE becomes ever so more a "complex" DSE and I wonder > if it would be better / possible to re-use DSEs dse_classify_store > (possibly with some > extra complexity parametrization).
I don't see any more changes to be done with this "simple" dse even anytime soon (tm). Except to maybe move it but that is part of bigger passes cleanup. Thanks, Andrew > > Thanks, > Richard. > > > PR tree-optimization/122247 > > > > gcc/ChangeLog: > > > > * tree-ssa-forwprop.cc (do_simple_agr_dse): Allow phi node for the > > usage > > if the usage of the phi result is just the "same" as the original > > clobber. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/tree-ssa/copy-prop-aggregate-sra-2.C: New test. > > > > Signed-off-by: Andrew Pinski <[email protected]> > > --- > > .../tree-ssa/copy-prop-aggregate-sra-2.C | 31 +++++++++++++++++++ > > gcc/tree-ssa-forwprop.cc | 13 ++++++++ > > 2 files changed, 44 insertions(+) > > create mode 100644 > > gcc/testsuite/g++.dg/tree-ssa/copy-prop-aggregate-sra-2.C > > > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/copy-prop-aggregate-sra-2.C > > b/gcc/testsuite/g++.dg/tree-ssa/copy-prop-aggregate-sra-2.C > > new file mode 100644 > > index 00000000000..0b05d5d03af > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/tree-ssa/copy-prop-aggregate-sra-2.C > > @@ -0,0 +1,31 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-forwprop1-details > > -fdump-tree-esra-details -fexceptions" } */ > > + > > +/* PR tree-optimization/122247 */ > > + > > +struct s1 > > +{ > > + int t[1024]; > > +}; > > + > > +struct s1 f(void); > > + > > +void g(int a, int b, int ); > > +void p(struct s1); > > +void h(struct s1 outer) > > +{ > > + struct s1 inner = outer; > > + p(inner); > > + g(outer.t[0], outer.t[1], outer.t[2]); > > +} > > +/* Forwprop should be able to copy prop the copy of `inner = outer` to the > > call of p. > > + Also remove this copy. */ > > + > > +/* { dg-final { scan-tree-dump-times "after previous" 1 "forwprop1" } } */ > > +/* { dg-final { scan-tree-dump-times "Removing dead store stmt inner = > > outer" 1 "forwprop1" } } */ > > + > > +/* The extra copy that was done by inlining is removed so SRA should not > > decide to cause > > + inner nor outer to be scalarized even for the 3 elements accessed > > afterwards. */ > > +/* { dg-final { scan-tree-dump-times "Disqualifying inner" 1 "esra" } } */ > > +/* { dg-final { scan-tree-dump-times "Disqualifying outer" 1 "esra" } } */ > > + > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > > index 0de0621c00a..71dacb8fbd4 100644 > > --- a/gcc/tree-ssa-forwprop.cc > > +++ b/gcc/tree-ssa-forwprop.cc > > @@ -1846,6 +1846,19 @@ do_simple_agr_dse (gassign *stmt, bool full_walk) > > if (gimple_clobber_p (use_stmt, kind) > > && lhs == gimple_assign_lhs (use_stmt)) > > continue; > > + /* If the use is a phi and it is single use then check if that > > single use > > + is a clobber of the same kind and lhs is the same. */ > > + if (gphi *use_phi = dyn_cast<gphi*>(use_stmt)) > > + { > > + use_operand_p ou; > > + gimple *ostmt; > > + if (single_imm_use (gimple_phi_result (use_phi), &ou, &ostmt) > > + && gimple_clobber_p (ostmt, kind) > > + && lhs == gimple_assign_lhs (ostmt)) > > + continue; > > + /* A phi node will never be dominating the clobber. */ > > + return; > > + } > > /* The use needs to be dominating the clobber. */ > > if ((ubb != bb && !dominated_by_p (CDI_DOMINATORS, bb, ubb)) > > || ref_maybe_used_by_stmt_p (use_stmt, &read, false)) > > -- > > 2.43.0 > >
