On Tue, Jul 14, 2026 at 1:47 AM Richard Biener <[email protected]> wrote: > > On Sat, Jul 4, 2026 at 5:10 AM 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 or a > > non-trapping (no data races or local variable) load right before > > the conditional. This allows for 2 things, the reuse of the same > > aliasing set and second is allows for use in phiopt in a secondary patch. > > > > It supports the inserting on the edge for the case where there is > > no load/stores before the condition if the store is a non-trapping memory > > with no data races; this is a similar to what is done in > > cond_store_replacement > > already. Thia part is not going to be used with phiopt though as it is too > > complex to support there and it adds a conditional load for no benifit for > > phiopt. > > > > 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. > > > > Changes since v1: > > * v2: Add support for loads before the conditional and support > > for doing the load on the edge. Fix formating and comments. > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > gcc/ChangeLog: > > > > * tree-ssa-phiopt.cc (may_alias_mem): New function. > > (can_trap_as_a_store): New funciton. > > (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 | 271 ++++++++++++++++++++++ > > 2 files changed, 272 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 e12dc7a8b0c..103547d538f 100644 > > --- a/gcc/tree-ssa-phiopt.cc > > +++ b/gcc/tree-ssa-phiopt.cc > > @@ -3386,6 +3386,272 @@ trailing_store_in_bb (basic_block bb, tree vdef, > > gphi *vphi, bool onlyonestore) > > return store; > > } > > > > +/* Takes a MEM and changes the aliasing set to be zero on it. > > + This handles all variants include decls. */ > > +static tree > > +may_alias_mem (tree mem) > > IMHO this is a bad name for the function, maybe > copy_mem_with_alias_set_zero?
Yes, that is definitely a better name.
>
> > +{
> > + mem = unshare_expr (mem);
> > + tree *basep = &mem;
> > + while (handled_component_p (*basep))
> > + basep = &TREE_OPERAND (*basep, 0);
> > + if (TREE_CODE (*basep) == MEM_REF
> > + || TREE_CODE (*basep) == TARGET_MEM_REF)
> > + TREE_OPERAND (*basep, 1)
> > + = fold_convert (ptr_type_node, TREE_OPERAND (*basep, 1));
> > + else
> > + *basep = build2 (MEM_REF, TREE_TYPE (*basep),
> > + build_fold_addr_expr (*basep),
> > + build_zero_cst (ptr_type_node));
> > + return mem;
> > +}
> > +
> > +/* Returns true if LHS is known not to trap as a store. */
> > +static bool
> > +can_trap_as_a_store (tree lhs)
>
> We can probably re-use this in LIM, maybe put it in
> tree-eh.cc alongside tree_could_trap_p and name
> it similarly, tree_could_trap_as_store_p or as
> lhs_could_trap_p?
Used lhs_could_trap_p.
Attached is what I pushed.
Thanks,
Andrea
>
> Otherwise looks good to me.
>
> Thanks,
> Richard.
>
>
> > +{
> > + tree lhsbase = get_base_address (lhs);
> > + if (tree_could_trap_p (lhs))
> > + return true;
> > + /* tree_could_trap_p is a predicate for loads, so check
> > + for readonly memory explicitly. */
> > + if ((DECL_P (lhsbase) && TREE_READONLY (lhsbase))
> > + || TREE_CODE (lhsbase) == STRING_CST)
> > + return true;
> > + return false;
> > +}
> > +
> > +/* 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,
> > + bool caninsert_edge)
> > +{
> > + 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)
> > + For case 2 and 3, check for data races.
> > + For case 2, the load can either be based on a local variable
> > + or a known non-trapping decl.
> > + For case 3, the store needs to known to be non-trapping. */
> > + tree vuse = gimple_vuse (store_middle);
> > + gimple *beforestore = nullptr;
> > + gimple *vdef_before = SSA_NAME_DEF_STMT (vuse);
> > + tree other_rhs = nullptr;
> > +
> > + /* See if there is a store before the condition case. */
> > + if (gimple_assign_single_p (vdef_before))
> > + {
> > + tree beforelhs = gimple_assign_lhs (vdef_before);
> > + /* Only allow the store to be right before the condition. */
> > + if (gimple_bb (vdef_before) == cond_bb
> > + /* An exact match is only supported.
> > + FIXME: Allow for clique/base mismatch? */
> > + && 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 and
> > + the previous store as that might depend on the store.
> > + FIXME: use alias oracle to check dependancies. */
> > + FOR_EACH_IMM_USE_STMT (use_stmt, iter, vuse)
> > + {
> > + if (use_stmt != store_middle
> > + && use_stmt != vphi)
> > + return false;
> > + }
> > + other_rhs = gimple_assign_rhs1 (vdef_before);
> > + beforestore = vdef_before;
> > + }
> > + }
> > + /*
> > + case 2:
> > + a = local_var[n];
> > + if (b)
> > + local_var[n] = c;
> > +
> > + case 3:
> > + if (b)
> > + nontrapping = c;
> > +
> > + For case 3, nontrapping needs to satisfy tree_could_trap_p.
> > + In both cases ref_can_have_store_data_races needs to be satisfy.
> > + */
> > +
> > + /* Maybe the load/local non-escaped variable case. */
> > + if (!other_rhs)
> > + {
> > + tree lhsbase = get_base_address (lhs);
> > + /* If this store ref can't have data races, a store
> > + that was conditional can't become unconditional. */
> > + if (ref_can_have_store_data_races (lhs))
> > + return false;
> > + tree vuse = gimple_vuse (store_middle);
> > + imm_use_iterator iter;
> > + gimple *use_stmt;
> > + /* Try to find the load before the store that matches
> > + if we have a local variable or a non trapping store. */
> > + if ((auto_var_p (lhsbase) && !TREE_ADDRESSABLE (lhsbase))
> > + || !can_trap_as_a_store (lhs))
> > + {
> > + FOR_EACH_IMM_USE_STMT (use_stmt, iter, vuse)
> > + {
> > + if (gimple_bb (use_stmt) != cond_bb)
> > + continue;
> > + /* Looking for a load only. */
> > + if (!gimple_assign_load_p (use_stmt))
> > + continue;
> > + tree rhs = gimple_assign_rhs1 (use_stmt);
> > + if (!operand_equal_p (rhs, lhs))
> > + continue;
> > + other_rhs = gimple_assign_lhs (use_stmt);
> > + lhs = may_alias_mem (lhs);
> > + break;
> > + }
> > + }
> > + if (!other_rhs)
> > + {
> > + gassign *new_stmt;
> > + /* If not allowing inserting on the edge, then don't. */
> > + if (!caninsert_edge)
> > + return false;
> > + /* If LHS is an access to a local variable without address-taken
> > + (or when we allow data races) and known not to trap, we could
> > + always safely move down the store. */
> > + if (can_trap_as_a_store (lhs))
> > + return false;
> > + lhs = may_alias_mem (lhs);
> > + /* Insert a load from the memory of the store to the temporary
> > + on the edge which did not contain the store. */
> > + other_rhs = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "cstore");
> > + new_stmt = gimple_build_assign (other_rhs, lhs);
> > + gimple_set_location (new_stmt, locus);
> > + /* Set the vuse for the new load. */
> > + gimple_set_vuse (new_stmt,
> > + gimple_phi_arg_def (vphi, e1->dest_idx));
> > + lhs = unshare_expr (lhs);
> > + {
> > + /* Set the no-warning bit on the rhs of the load to avoid
> > + uninit warnings. */
> > + tree rhs1 = gimple_assign_rhs1 (new_stmt);
> > + suppress_warning (rhs1, OPT_Wuninitialized);
> > + }
> > + gsi_insert_on_edge (e1, new_stmt);
> > + }
> > + }
> > +
> > + 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 +4898,11 @@ pass_cselim::execute (function *)
> > if (EDGE_COUNT (bb2->preds) > 2)
> > return;
> >
> > + if (cond_store_replacement_limited (bb1, bb2, bb, e1, e2, true))
> > + {
> > + cfgchanged = true;
> > + return;
> > + }
> > gimple *assign = cselim_candidate (bb1, bb2, e1);
> > if (cond_store_replacement (bb1, bb2, e1, e2, assign, nontrap))
> > cfgchanged = true;
> > --
> > 2.43.0
> >
0001-cselim-reduced-cs-elim-without-non-trapping.patch
Description: Binary data
