https://gcc.gnu.org/g:391ee229b737eb9a0cf3b705f41476f56d12d1e3
commit r17-1273-g391ee229b737eb9a0cf3b705f41476f56d12d1e3 Author: Pengxuan Zheng <[email protected]> Date: Wed Apr 8 18:09:48 2026 -0700 cselim: Allow middle_bb to have more than one statement [PR124405] 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 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]> Diff: --- gcc/testsuite/gcc.dg/tree-ssa/pr124405.c | 12 ++++++++ gcc/tree-ssa-phiopt.cc | 49 ++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 12 deletions(-) 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 000000000000..9ba230d2b56a --- /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 1bdb37092990..6a7e4c70722a 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; };
