[gcc r17-2634] cselim: reduced cs-elim without non-trapping
Andrea Pinski via Gcc-cvs <[email protected]>
| Newsgroups | gmane.comp.gcc.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://gcc.gnu.org/g:fd3e590360dc4f479e69142ed1bd6775c07213b6 commit r17-2634-gfd3e590360dc4f479e69142ed1bd6775c07213b6 Author: Andrew Pinski <[email protected]> Date: Sat Jun 20 23:01:07 2026 -0700 cselim: reduced cs-elim without non-trapping 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. * v3: Move and rename can_trap_as_a_store to tree-eh.cc (lhs_could_trap_p). Rename may_alias_mem to copy_mem_with_alias_set_zero. Bootstrapped and tested on x86_64-linux-gnu. gcc/ChangeLog: * tree-eh.cc (lhs_could_trap_p): New function. * tree-eh.h (lhs_could_trap_p): New decl. * tree-ssa-phiopt.cc (copy_mem_with_alias_set_zero): New function. (cond_store_replacement_limited): New function. (pass_cselim::execute): Call cond_store_replacement_limited before 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: Andrea Pinski <[email protected]> Diff: --- gcc/testsuite/gcc.dg/tree-ssa/pr99473-1.c | 2 +- gcc/tree-eh.cc | 15 ++ gcc/tree-eh.h | 1 + gcc/tree-ssa-phiopt.cc | 256 ++++++++++++++++++++++++++++++ 4 files changed, 273 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 0fda5663a801..e930389e9c0a 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-eh.cc b/gcc/tree-eh.cc index 2980da229980..0835f55af086 100644 --- a/gcc/tree-eh.cc +++ b/gcc/tree-eh.cc @@ -2918,6 +2918,21 @@ tree_could_trap_p (tree expr) } } +/* Returns true if LHS is known not to trap as a store. */ +bool +lhs_could_trap_p (tree lhs) +{ + 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; +} + /* Return non-NULL if there is an integer operation with trapping overflow we can rewrite into non-trapping. Called via walk_tree from rewrite_to_non_trapping_overflow. */ diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h index 96c5ec4aefca..670f377a2907 100644 --- a/gcc/tree-eh.h +++ b/gcc/tree-eh.h @@ -42,6 +42,7 @@ extern tree rewrite_to_non_trapping_overflow (tree); extern bool stmt_could_throw_p (function *, gimple *); extern bool stmt_unremovable_because_of_non_call_eh_p (function *, gimple *); extern bool tree_could_throw_p (tree); +extern bool lhs_could_trap_p (tree); extern bool stmt_can_throw_external (function *, gimple *); extern bool stmt_can_throw_internal (function *, gimple *); extern bool maybe_clean_eh_stmt_fn (struct function *, gimple *); diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index e12dc7a8b0ca..054c025f8829 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -3386,6 +3386,257 @@ 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 +copy_mem_with_alias_set_zero (tree mem) +{ + 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; +} + +/* 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)) + || !lhs_could_trap_p (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 = copy_mem_with_alias_set_zero (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 (lhs_could_trap_p (lhs)) + return false; + lhs = copy_mem_with_alias_set_zero (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 +4883,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;