On Thu, Aug 27, 2026 at 1:31 AM Andrea Pinski <[email protected]> wrote: > > I noticed this while working on std::optional, the full blown cselim with > Data references > hanldes the case where there are a few stores on either side of the branch > that don't > interfer with the other stores. But we can handle the limited case where the > last "store" > on one of the branches was a clobber. Since the clobber is not a real store > it can > be "safely" bypassed without checking the lhs. > > This was found by looking into std::optional::reset code gen.
Don't you need to check whether the clobber clobbers the stored object you want to sink a store to? > Bootstrapped and tested on x86_64-linux-gnu. > > PR tree-optimization/127053 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (trailing_store_in_bb): Rename vphi to > vuse_only. > (cond_if_else_store_replacement_limited): Add case if > cond_if_else_store_replacement_1 failed for if one > of the stores were a clobber. > > gcc/testsuite/ChangeLog: > > * g++.dg/tree-ssa/cselim-2.C: New test. > > Signed-off-by: Andrea Pinski <[email protected]> > --- > gcc/testsuite/g++.dg/tree-ssa/cselim-2.C | 23 ++++++++++++++++ > gcc/tree-ssa-phiopt.cc | 34 +++++++++++++++++++----- > 2 files changed, 51 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cselim-2.C > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/cselim-2.C > b/gcc/testsuite/g++.dg/tree-ssa/cselim-2.C > new file mode 100644 > index 00000000000..9c040f9d183 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/tree-ssa/cselim-2.C > @@ -0,0 +1,23 @@ > +// PR tree-optimization/127053 > +// { dg-do compile } > +// { dg-options "-O2 -ftree-cselim -fdump-tree-phiopt1-details" } > +// testcase reduced from std::optional::reset. > + > +void sink(int*); > +typedef int T; > + > +int f(int b, T *c) > +{ > + int a; > + sink(&a); > + if (b) > + { > + a = 0; > + c->~T(); > + } > + else > + a = 0; > + return a; > +} > + > +// { dg-final { scan-tree-dump "factoring out stores" "phiopt1" } } > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index ddb2e353cf4..14a6b8262bb 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -3547,12 +3547,12 @@ cond_if_else_store_replacement_1 (basic_block > then_bb, basic_block else_bb, > } > > /* Return the last store in BB with VDEF or NULL if there are > - loads following the store. VPHI is where the only use of the > + loads following the store. VUSE_ONLY is where the only use of the > vdef should be. If ONLYONESTORE is true, then the store is > the only store in the BB. */ > > static gimple * > -trailing_store_in_bb (basic_block bb, tree vdef, gphi *vphi, bool > onlyonestore) > +trailing_store_in_bb (basic_block bb, tree vdef, gimple *vuse_only, bool > onlyonestore) > { > if (SSA_NAME_IS_DEFAULT_DEF (vdef)) > return NULL; > @@ -3570,12 +3570,12 @@ trailing_store_in_bb (basic_block bb, tree vdef, gphi > *vphi, bool onlyonestore) > > > /* Verify there is no load or store after the store, the vdef of the store > - should only be used by the vphi joining the 2 bbs. */ > + should only be used by the vuse_only. */ > use_operand_p use_p; > gimple *use_stmt; > if (!single_imm_use (gimple_vdef (store), &use_p, &use_stmt)) > return NULL; > - if (use_stmt != vphi) > + if (use_stmt != vuse_only) > return NULL; > > return store; > @@ -3900,8 +3900,30 @@ cond_if_else_store_replacement_limited (basic_block > then_bb, basic_block else_bb > if (!else_assign) > return false; > > - return cond_if_else_store_replacement_1 (then_bb, else_bb, join_bb, > - then_assign, else_assign, vphi); > + if (!cond_if_else_store_replacement_1 (then_bb, else_bb, join_bb, > + then_assign, else_assign, vphi)) > + { > + if (!flag_expensive_optimizations > + || gimple_clobber_p (then_assign) == gimple_clobber_p (else_assign)) > + return false; > + if (gimple_clobber_p (then_assign)) > + { > + then_assign = trailing_store_in_bb (then_bb, gimple_vuse > (then_assign), > + then_assign, true); > + if (!then_assign) > + return false; > + } > + else > + { > + else_assign = trailing_store_in_bb (else_bb, gimple_vuse > (else_assign), > + else_assign, true); > + if (!else_assign) > + return false; > + } > + return cond_if_else_store_replacement_1 (then_bb, else_bb, join_bb, > + then_assign, else_assign, > vphi); > + } > + return true; > } > > /* Conditional store replacement. We already know > -- > 2.43.0 >
