On Wed, Jun 3, 2026 at 5:08 AM Richard Biener <[email protected]> wrote:
> On Sat, May 23, 2026 at 4:04 AM Pengxuan Zheng > <[email protected]> wrote: > > > > Currently, cselim requires the middle_bb to have only a single > statement. This > > patch relaxes this restriction as long as all uses (except the rhs) in > the > > single stmatement are also available where we insert to. > > > > Bootstrapped and tested on x86_64-linux-gnu and aarch64-linux-gnu. > > > > Changes since v1: > > * v2: Revert changes to trailing_store_in_bb and do not call > > trailing_store_in_bb to get split_assign. > > Remove some unnecessary checks in cselim_candidate. > > Pass the result of cselim_candidate which is the candidate store > for > > cselim as an argument to cond_store_replacement instead. > > * v3: Simplify cselim_candidate by always calling trailing_store_in_bb > and > > removing the legality checks. > > Generalize the cond_store_replacement legality checks to account > for the > > case when the middle_bb can have more than one statement. > > > > PR tree-optimization/124405 > > OK. > Thanks, Richard. I've pushed the change as r17-1273-g391ee229b. Pengxuan > > Thanks, > Richard. > > > gcc/ChangeLog: > > > > * tree-ssa-phiopt.cc (cond_store_replacement): Make ASSIGN (the > > candidate store for cselim) an argument of the function instead. > Update > > legality check due to relaxing the restriction that middle_bb > can have > > only a single statement. > > (cselim_candidate): New. > > (pass_cselim::execute): Call cselim_candidate and pass the > result to > > cond_store_replacement. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/pr124405.c: New test. > > > > Signed-off-by: Pengxuan Zheng <[email protected]> > > --- > > gcc/testsuite/gcc.dg/tree-ssa/pr124405.c | 12 ++++++ > > gcc/tree-ssa-phiopt.cc | 49 ++++++++++++++++++------ > > 2 files changed, 49 insertions(+), 12 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr124405.c > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr124405.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr124405.c > > new file mode 100644 > > index 00000000000..9ba230d2b56 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr124405.c > > @@ -0,0 +1,12 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O1 -fdump-tree-cselim-details" } */ > > + > > +void > > +f (int *a, int b) > > +{ > > + *a &= ~3; > > + if (b) > > + *a |= 1; > > +} > > + > > +/* { dg-final { scan-tree-dump-times "Conditional store replacement > happened" 1 "cselim"} } */ > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > > index 18b5f284eee..d0e5f898eb4 100644 > > --- a/gcc/tree-ssa-phiopt.cc > > +++ b/gcc/tree-ssa-phiopt.cc > > @@ -2999,16 +2999,16 @@ get_non_trapping (void) > > JOIN_BB: > > some more > > > > - We check that MIDDLE_BB contains only one store, that that store > > + ASSIGN is a store in MIDDLE_BB which is the candidate for cselim. > We check > > + that MIDDLE_BB contains only one store (i.e., ASSIGN), that that > store > > doesn't trap (not via NOTRAP, but via checking if an access to the > same > > - memory location dominates us, or the store is to a local addressable > > - object) and that the store has a "simple" RHS. */ > > + memory location dominates us, or the store is to a local addressable > object) > > + and that the store has a "simple" RHS. */ > > > > static bool > > -cond_store_replacement (basic_block middle_bb, basic_block join_bb, > > - edge e0, edge e1, hash_set<tree> *nontrap) > > +cond_store_replacement (basic_block middle_bb, basic_block join_bb, > edge e0, > > + edge e1, gimple *assign, hash_set<tree> *nontrap) > > { > > - gimple *assign = last_and_only_stmt (middle_bb); > > tree lhs, rhs, name, name2; > > gphi *newphi; > > gassign *new_stmt; > > @@ -3021,11 +3021,6 @@ cond_store_replacement (basic_block middle_bb, > basic_block join_bb, > > || gimple_has_volatile_ops (assign)) > > return false; > > > > - /* And no PHI nodes so all uses in the single stmt are also > > - available where we insert to. */ > > - if (!gimple_seq_empty_p (phi_nodes (middle_bb))) > > - return false; > > - > > locus = gimple_location (assign); > > lhs = gimple_assign_lhs (assign); > > rhs = gimple_assign_rhs1 (assign); > > @@ -3034,6 +3029,20 @@ cond_store_replacement (basic_block middle_bb, > basic_block join_bb, > > || !is_gimple_reg_type (TREE_TYPE (lhs))) > > return false; > > > > + /* Make sure all uses (except the rhs) in the single stmt are also > available > > + where we insert to. */ > > + ssa_op_iter iter; > > + tree use; > > + FOR_EACH_SSA_TREE_OPERAND (use, assign, iter, SSA_OP_USE) > > + { > > + if (use == rhs) > > + continue; > > + > > + gimple *stmt = SSA_NAME_DEF_STMT (use); > > + if (stmt && gimple_bb (stmt) == middle_bb) > > + return false; > > + } > > + > > /* Prove that we can move the store down. We could also check > > TREE_THIS_NOTRAP here, but in that case we also could move stores, > > whose value is not available readily, which we want to avoid. */ > > @@ -3297,6 +3306,20 @@ trailing_store_in_bb (basic_block bb, tree vdef, > gphi *vphi, bool onlyonestore) > > return store; > > } > > > > +/* Return the only store in MIDDLE_BB as the candidate store for > cselim. Return > > + NULL if no candidate can be found. */ > > + > > +static gimple * > > +cselim_candidate (basic_block middle_bb, basic_block join_bb, edge e0) > > +{ > > + gphi *vphi = get_virtual_phi (join_bb); > > + if (!vphi) > > + return NULL; > > + > > + tree middle_vdef = PHI_ARG_DEF_FROM_EDGE (vphi, e0); > > + return trailing_store_in_bb (middle_bb, middle_vdef, vphi, true); > > +} > > + > > /* Limited Conditional store replacement. We already know > > that the recognized pattern looks like so: > > > > @@ -4268,7 +4291,9 @@ pass_cselim::execute (function *) > > optimization if the join block has more than two predecessors. > */ > > if (EDGE_COUNT (bb2->preds) > 2) > > return; > > - if (cond_store_replacement (bb1, bb2, e1, e2, nontrap)) > > + > > + gimple *assign = cselim_candidate (bb1, bb2, e1); > > + if (cond_store_replacement (bb1, bb2, e1, e2, assign, nontrap)) > > cfgchanged = true; > > }; > > > > -- > > 2.34.1 > > >
