On Mon, Jun 22, 2026 at 9:35 PM Andrew Pinski <[email protected]> wrote: > > This adds a reduced cs-elim functionality which will be used > from phiopt. The design is similar to cond_store_replacement except > we are looking for a similar store right beore the condition. > This allows for 2 things, the reuse of the same aliasing set and > second is allows for use in phiopt in a secondary patch. > > This can and will be expanded to support the case where there is a > similar load as the store before the condition for the cases that > is allowed, e.g. local non-addressable variables or decls when allowing > race conditions. > > Note pr99473-1.c of a case where can be optimized even without > -fallow-store-data-races now. The store happens before the conditional > and inside the conditional, so afterwards there is only one store rather > than 2. The code was also handling it in sink in GCC 12 the same way too > but now handling it in cselim before sink1.
This should have been sent out as a RFC rather than fully proposing the patch. This is the first step on deciding if the rest of cs-elim is needed or not. Thanks, Andrea > > Bootstrapped and tested on x86_64-linux-gnu. > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (cond_store_replacement_limited): New function. > (pass_cselim::execute): Call cond_store_replacement_limited before > cond_store_replacement. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/pr99473-1.c: Remove -fallow-store-data-races as it > is not > needed with cond_store_replacement_limited. > > Signed-off-by: Andrew Pinski <[email protected]> > --- > gcc/testsuite/gcc.dg/tree-ssa/pr99473-1.c | 2 +- > gcc/tree-ssa-phiopt.cc | 163 ++++++++++++++++++++++ > 2 files changed, 164 insertions(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr99473-1.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr99473-1.c > index 0fda5663a80..e930389e9c0 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/pr99473-1.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr99473-1.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -ftree-cselim -fallow-store-data-races > -fdump-tree-cselim-details" } */ > +/* { dg-options "-O2 -ftree-cselim -fdump-tree-cselim-details" } */ > > void f (int*); > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index 5c263df70eb..27f1d60aa46 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -3386,6 +3386,167 @@ trailing_store_in_bb (basic_block bb, tree vdef, gphi > *vphi, bool onlyonestore) > return store; > } > > + > +/* Do the main work of a limited conditional store replacement. > + This recognized pattern like so: > + > + COND_BB: > + store = a_1; > + // no loads > + if (cond) goto MIDDLE_BB; else goto JOIN_BB (edge E1) > + MIDDLE_BB: > + something // no loads > + store = a_2; > + something // no loads > + fallthrough (edge E0) > + JOIN_BB: > + some more > + > + This is a limited form of the full cond_store_replacement > + to be allowed from use from phiopt and can be done > + without calculating the non-trapping cases. */ > +static bool > +cond_store_replacement_limited (basic_block middle_bb, basic_block join_bb, > + basic_block cond_bb, > + edge e0, edge e1) > +{ > + tree lhs, rhs; > + location_t locus; > + /* Currently don't handle more than 2 incoming edges > + into the merge bb. */ > + if (EDGE_COUNT (join_bb->preds) > 2) > + return false; > + /* The middle bb needs to have a single predecessor of the cond_bb. */ > + if (!single_pred_p (middle_bb)) > + return false; > + gphi *vphi = get_virtual_phi (join_bb); > + if (!vphi) > + return false; > + tree middle_vdef = gimple_phi_arg_def_from_edge (vphi, e0); > + /* Check if middle_bb contains of only one store. */ > + gimple *store_middle; > + store_middle = trailing_store_in_bb (middle_bb, middle_vdef, > + vphi, true); > + > + if (!store_middle > + || !gimple_assign_single_p (store_middle) > + || gimple_has_volatile_ops (store_middle)) > + return false; > + > + locus = gimple_location (store_middle); > + lhs = gimple_assign_lhs (store_middle); > + rhs = gimple_assign_rhs1 (store_middle); > + if ((!REFERENCE_CLASS_P (lhs) > + && !DECL_P (lhs)) > + || !is_gimple_reg_type (TREE_TYPE (lhs))) > + return false; > + if (TREE_CODE (rhs) != SSA_NAME) > + return false; > + > + /* Three cases that can be handled: > + 1) the lhs is stored to right before the condition. > + Will remove the store before the condition. > + 2) Or the lhs is loaded from right before the condition. > + 3) Neither of these. (this will insert a load in the other edge) > + > + 2 needs: auto_var_p (base) && !TREE_ADDRESSABLE (base) > + or !ref_can_have_store_data_races > + && !((DECL_P (base) && TREE_READONLY (base)) > + || TREE_CODE (base) == STRING_CST) > + && !tree_could_trap_p > + 3 needs !ref_can_have_store_data_races > + && !((DECL_P (base) && TREE_READONLY (base)) > + || TREE_CODE (base) == STRING_CST) > + && !tree_could_trap_p > + FIXME: Only case 1 is handled currently. */ > + tree vuse = gimple_vuse (store_middle); > + gimple *beforestore = SSA_NAME_DEF_STMT (vuse); > + tree other_rhs = nullptr; > + > + /* Maybe the store case. */ > + if (gimple_assign_single_p (beforestore)) > + { > + tree beforelhs = gimple_assign_lhs (beforestore); > + if (gimple_bb (beforestore) == cond_bb > + && operand_equal_p (lhs, beforelhs)) > + { > + /* The vuse of the of store in the middle should be also > + the entry in the phi for the other edge. */ > + gcc_assert (vuse == gimple_phi_arg_def_from_edge (vphi, e1)); > + tree vuse = gimple_vuse (store_middle); > + imm_use_iterator iter; > + gimple *use_stmt; > + /* There can't be any loads between the store > + the previous store. */ > + FOR_EACH_IMM_USE_STMT (use_stmt, iter, vuse) > + { > + if (use_stmt != store_middle > + && use_stmt != vphi) > + return false; > + } > + other_rhs = gimple_assign_rhs1 (beforestore); > + } > + } > + // FIXME: handle more than case 1. > + // Maybe the load/local non-escaped variable case. > + if (!other_rhs) > + return false; > + > + gphi *newphi; > + gassign *new_stmt; > + gimple_stmt_iterator gsi; > + /* Now we've checked the constraints, so do the transformation: > + 1) Remove the store(s). */ > + gsi = gsi_for_stmt (store_middle); > + unlink_stmt_vdef (store_middle); > + gsi_remove (&gsi, true); > + release_defs (store_middle); > + > + if (beforestore) > + { > + gsi = gsi_for_stmt (beforestore); > + unlink_stmt_vdef (beforestore); > + gsi_remove (&gsi, true); > + release_defs (beforestore); > + } > + > + > + /* 2) Create a PHI node at the join block, with one argument > + holding the old RHS, and the other holding the temporary > + where we stored the old memory contents. */ > + tree phiname = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "cstore"); > + newphi = create_phi_node (phiname, join_bb); > + add_phi_arg (newphi, rhs, e0, locus); > + add_phi_arg (newphi, other_rhs, e1, locus); > + > + /* 3. Create the new store. */ > + new_stmt = gimple_build_assign (lhs, phiname); > + > + /* Update the vdef for the new store statement. */ > + tree newvphilhs = make_ssa_name (gimple_vop (cfun)); > + tree vdef = gimple_phi_result (vphi); > + gimple_set_vuse (new_stmt, newvphilhs); > + gimple_set_vdef (new_stmt, vdef); > + gimple_phi_set_result (vphi, newvphilhs); > + SSA_NAME_DEF_STMT (vdef) = new_stmt; > + update_stmt (vphi); > + > + gsi = gsi_after_labels (join_bb); > + gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); > + > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, "\nConditional store replacement happened!"); > + if (beforestore) > + fprintf (dump_file, "\nRemoved the store before the condition."); > + fprintf (dump_file, "\nInserted a new PHI statement in joint > block:\n"); > + print_gimple_stmt (dump_file, new_stmt, 0, TDF_VOPS|TDF_MEMSYMS); > + } > + statistics_counter_event (cfun, "conditional store replacement", 1); > + return true; > +} > + > + > /* Return the only store in MIDDLE_BB as the candidate store for cselim. > Return > NULL if no candidate can be found. */ > > @@ -4632,6 +4793,8 @@ pass_cselim::execute (function *) > if (EDGE_COUNT (bb2->preds) > 2) > return; > > + if (cond_store_replacement_limited (bb1, bb2, bb, e1, e2)) > + cfgchanged = true; > gimple *assign = cselim_candidate (bb1, bb2, e1); > if (cond_store_replacement (bb1, bb2, e1, e2, assign, nontrap)) > cfgchanged = true; > -- > 2.43.0 >
