On Sun, Nov 16, 2025 at 7:05 PM Andrew Pinski <[email protected]> wrote: > > remove_forwarder_block is never called without calling tree_forwarder_block_p > before hand so this merges the two functions and simplifies the code slightly. > tree_forwarder_block_p was only returning true at the very end (except for one > location in the code checking the loop which is fixed here) so this nature to > merge the two functions even. > > move_debug_stmts_from_forwarder and phi_alternatives_equal definitions were > moved > to before maybe_remove_forwarder_block. > > Bootstrapped and tested on x86_64-linux-gnu.
OK. > gcc/ChangeLog: > > * tree-cfgcleanup.cc (tree_forwarder_block_p): Merge this and ... > (remove_forwarder_block): This into ... > (maybe_remove_forwarder_block): Here. > (cleanup_tree_cfg_bb): Call only maybe_remove_forwarder_block. > (pass_merge_phi::execute): Likewise. > > Signed-off-by: Andrew Pinski <[email protected]> > --- > gcc/tree-cfgcleanup.cc | 207 ++++++++++++++++++++--------------------- > 1 file changed, 100 insertions(+), 107 deletions(-) > > diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc > index 9aeb4f3fc99..b9384db9f3d 100644 > --- a/gcc/tree-cfgcleanup.cc > +++ b/gcc/tree-cfgcleanup.cc > @@ -378,15 +378,103 @@ cleanup_control_flow_bb (basic_block bb) > return retval; > } > > + > +/* If all the PHI nodes in DEST have alternatives for E1 and E2 and > + those alternatives are equal in each of the PHI nodes, then return > + true, else return false. */ > + > +bool > +phi_alternatives_equal (basic_block dest, edge e1, edge e2) > +{ > + int n1 = e1->dest_idx; > + int n2 = e2->dest_idx; > + gphi_iterator gsi; > + > + for (gsi = gsi_start_phis (dest); !gsi_end_p (gsi); gsi_next (&gsi)) > + { > + gphi *phi = gsi.phi (); > + tree val1 = gimple_phi_arg_def (phi, n1); > + tree val2 = gimple_phi_arg_def (phi, n2); > + > + gcc_assert (val1 != NULL_TREE); > + gcc_assert (val2 != NULL_TREE); > + > + if (!operand_equal_for_phi_arg_p (val1, val2)) > + return false; > + } > + > + return true; > +} > + > +/* Move debug stmts from the forwarder block SRC to DEST or PRED. */ > + > +static void > +move_debug_stmts_from_forwarder (basic_block src, > + basic_block dest, bool dest_single_pred_p, > + basic_block pred, bool pred_single_succ_p) > +{ > + if (!MAY_HAVE_DEBUG_STMTS) > + return; > + > + /* If we cannot move to the destination but to the predecessor do that. */ > + if (!dest_single_pred_p && pred_single_succ_p) > + { > + gimple_stmt_iterator gsi_to = gsi_last_bb (pred); > + if (gsi_end_p (gsi_to) || !stmt_ends_bb_p (gsi_stmt (gsi_to))) > + { > + for (gimple_stmt_iterator gsi = gsi_after_labels (src); > + !gsi_end_p (gsi);) > + { > + gimple *debug = gsi_stmt (gsi); > + gcc_assert (is_gimple_debug (debug)); > + gsi_move_after (&gsi, &gsi_to); > + } > + return; > + } > + } > + > + /* Else move to DEST or drop/reset them. */ > + gimple_stmt_iterator gsi_to = gsi_after_labels (dest); > + for (gimple_stmt_iterator gsi = gsi_after_labels (src); !gsi_end_p (gsi);) > + { > + gimple *debug = gsi_stmt (gsi); > + gcc_assert (is_gimple_debug (debug)); > + /* Move debug binds anyway, but not anything else like begin-stmt > + markers unless they are always valid at the destination. */ > + if (dest_single_pred_p > + || gimple_debug_bind_p (debug)) > + { > + gsi_move_before (&gsi, &gsi_to); > + /* Reset debug-binds that are not always valid at the destination. > + Simply dropping them can cause earlier values to become live, > + generating wrong debug information. > + ??? There are several things we could improve here. For > + one we might be able to move stmts to the predecessor. > + For anther, if the debug stmt is immediately followed by a > + (debug) definition in the destination (on a post-dominated path?) > + we can elide it without any bad effects. */ > + if (!dest_single_pred_p) > + { > + gimple_debug_bind_reset_value (debug); > + update_stmt (debug); > + } > + } > + else > + gsi_next (&gsi); > + } > +} > + > /* Return true if basic block BB does nothing except pass control > flow to another block and that we can safely insert a label at > - the start of the successor block. > + the start of the successor block and was removed. > > As a precondition, we require that BB be not equal to > - the entry block. */ > + the entry block. > + If CAN_SPLIT is true, we can split the edge to have > + another bb with with the phi. */ > > static bool > -tree_forwarder_block_p (basic_block bb) > +maybe_remove_forwarder_block (basic_block bb, bool can_split = false) > { > gimple_stmt_iterator gsi; > location_t locus; > @@ -460,11 +548,12 @@ tree_forwarder_block_p (basic_block bb) > } > } > > + bool has_phi = !gimple_seq_empty_p (phi_nodes (bb)); > /* If BB has PHIs and does not dominate DEST, > then the PHI nodes at DEST must be the only > users of the results of the PHI nodes at BB. > So only check when BB dominates dest. */ > - if (!gimple_seq_empty_p (phi_nodes (bb)) > + if (has_phi > && dominated_by_p (CDI_DOMINATORS, dest, bb)) > { > gphi_iterator gsi; > @@ -513,9 +602,10 @@ tree_forwarder_block_p (basic_block bb) > /* If bb doesn't have a single predecessor we'd make this > loop have multiple latches. Don't do that if that > would in turn require disambiguating them. */ > - return (single_pred_p (bb) > - || loops_state_satisfies_p > - (LOOPS_MAY_HAVE_MULTIPLE_LATCHES)); > + if (!single_pred_p (bb) > + && !loops_state_satisfies_p > + (LOOPS_MAY_HAVE_MULTIPLE_LATCHES)) > + return false; > } > /* cleanup_tree_cfg_noloop just created the loop preheader, don't > remove it if it has phis. */ > @@ -530,104 +620,9 @@ tree_forwarder_block_p (basic_block bb) > } > } > > - return true; > -} > - > -/* If all the PHI nodes in DEST have alternatives for E1 and E2 and > - those alternatives are equal in each of the PHI nodes, then return > - true, else return false. */ > - > -bool > -phi_alternatives_equal (basic_block dest, edge e1, edge e2) > -{ > - int n1 = e1->dest_idx; > - int n2 = e2->dest_idx; > - gphi_iterator gsi; > - > - for (gsi = gsi_start_phis (dest); !gsi_end_p (gsi); gsi_next (&gsi)) > - { > - gphi *phi = gsi.phi (); > - tree val1 = gimple_phi_arg_def (phi, n1); > - tree val2 = gimple_phi_arg_def (phi, n2); > - > - gcc_assert (val1 != NULL_TREE); > - gcc_assert (val2 != NULL_TREE); > - > - if (!operand_equal_for_phi_arg_p (val1, val2)) > - return false; > - } > - > - return true; > -} > - > -/* Move debug stmts from the forwarder block SRC to DEST or PRED. */ > - > -static void > -move_debug_stmts_from_forwarder (basic_block src, > - basic_block dest, bool dest_single_pred_p, > - basic_block pred, bool pred_single_succ_p) > -{ > - if (!MAY_HAVE_DEBUG_STMTS) > - return; > - > - /* If we cannot move to the destination but to the predecessor do that. */ > - if (!dest_single_pred_p && pred_single_succ_p) > - { > - gimple_stmt_iterator gsi_to = gsi_last_bb (pred); > - if (gsi_end_p (gsi_to) || !stmt_ends_bb_p (gsi_stmt (gsi_to))) > - { > - for (gimple_stmt_iterator gsi = gsi_after_labels (src); > - !gsi_end_p (gsi);) > - { > - gimple *debug = gsi_stmt (gsi); > - gcc_assert (is_gimple_debug (debug)); > - gsi_move_after (&gsi, &gsi_to); > - } > - return; > - } > - } > - > - /* Else move to DEST or drop/reset them. */ > - gimple_stmt_iterator gsi_to = gsi_after_labels (dest); > - for (gimple_stmt_iterator gsi = gsi_after_labels (src); !gsi_end_p (gsi);) > - { > - gimple *debug = gsi_stmt (gsi); > - gcc_assert (is_gimple_debug (debug)); > - /* Move debug binds anyway, but not anything else like begin-stmt > - markers unless they are always valid at the destination. */ > - if (dest_single_pred_p > - || gimple_debug_bind_p (debug)) > - { > - gsi_move_before (&gsi, &gsi_to); > - /* Reset debug-binds that are not always valid at the destination. > - Simply dropping them can cause earlier values to become live, > - generating wrong debug information. > - ??? There are several things we could improve here. For > - one we might be able to move stmts to the predecessor. > - For anther, if the debug stmt is immediately followed by a > - (debug) definition in the destination (on a post-dominated path?) > - we can elide it without any bad effects. */ > - if (!dest_single_pred_p) > - { > - gimple_debug_bind_reset_value (debug); > - update_stmt (debug); > - } > - } > - else > - gsi_next (&gsi); > - } > -} > - > -/* Removes forwarder block BB. Returns false if this failed. */ > - > -static bool > -remove_forwarder_block (basic_block bb, bool can_split = false) > -{ > edge succ = single_succ_edge (bb), e, s; > - basic_block dest = succ->dest; > gimple *stmt; > - gimple_stmt_iterator gsi, gsi_to; > - bool has_phi = !gimple_seq_empty_p (phi_nodes (bb)); > + gimple_stmt_iterator gsi_to; > > /* If there is an abnormal edge to basic block BB, but not into > dest, problems might occur during removal of the phi node at out > @@ -872,8 +867,7 @@ want_merge_blocks_p (basic_block bb1, basic_block bb2) > static bool > cleanup_tree_cfg_bb (basic_block bb) > { > - if (tree_forwarder_block_p (bb) > - && remove_forwarder_block (bb)) > + if (maybe_remove_forwarder_block (bb)) > return true; > > /* If there is a merge opportunity with the predecessor > @@ -1384,8 +1378,7 @@ pass_merge_phi::execute (function *fun) > continue; > > /* Look for a forwarder block with PHI nodes. */ > - if (tree_forwarder_block_p (bb) > - && remove_forwarder_block (bb, true)) > + if (maybe_remove_forwarder_block (bb, true)) > forwarder_removed++; > } > > -- > 2.43.0 >
