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