On Thu, Nov 13, 2025 at 5:25 AM Richard Biener <[email protected]> wrote: > > On Thu, Nov 13, 2025 at 6:02 AM Andrew Pinski > <[email protected]> wrote: > > > > This moves the checks that were in pass_merge_phi::execute into > > remove_forwarder_block_with_phi > > or tree_forwarder_block_p to make easier to merge > > remove_forwarder_block_with_phi with remove_forwarder_block. > > This also simplifies the code slightly because we can do `return false` > > rather than break > > in one location. > > So this puts back one O(num-stmts) loop into CFG cleanup which I > previously tried > to make sure to be O(CFG size). But looking we're already doing > O(#debug-stmts + #labels) > work here, so this is a moot point.
There is also phi_alternatives_equal check in some cases too. Also this is O(num-phis) rather than O(num-stmts) which is slightly better. I didn't change the code to use a phi iterator; it still uses the gsi iterator. This also raises a good point, the phi iterator is not used in many places (and I always forgot it exists); maybe we should change gsi iterators not to work over phis but maybe that is a lot of work fixing all of the current code. Thanks, Andrew > > So OK to this (and the previous in the series). > Richard. > > > gcc/ChangeLog: > > > > * tree-cfgcleanup.cc (pass_merge_phi::execute): Move > > check for abnormal or no phis to remove_forwarder_block_with_phi > > and the check on dominated to tree_forwarder_block_p. > > (remove_forwarder_block_with_phi): here. > > > > Signed-off-by: Andrew Pinski <[email protected]> > > --- > > gcc/tree-cfgcleanup.cc | 102 +++++++++++++++++++---------------------- > > 1 file changed, 48 insertions(+), 54 deletions(-) > > > > diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc > > index 2f4fa0a7b8a..9f526492b72 100644 > > --- a/gcc/tree-cfgcleanup.cc > > +++ b/gcc/tree-cfgcleanup.cc > > @@ -456,6 +456,43 @@ tree_forwarder_block_p (basic_block bb, bool > > phi_wanted) > > } > > } > > > > + /* 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)) > > + && dominated_by_p (CDI_DOMINATORS, dest, bb)) > > + { > > + gphi_iterator gsi; > > + unsigned int dest_idx = single_succ_edge (bb)->dest_idx; > > + > > + /* BB dominates DEST. There may be many users of the PHI > > + nodes in BB. However, there is still a trivial case we > > + can handle. If the result of every PHI in BB is used > > + only by a PHI in DEST, then we can trivially merge the > > + PHI nodes from BB into DEST. */ > > + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); > > + gsi_next (&gsi)) > > + { > > + gphi *phi = gsi.phi (); > > + tree result = gimple_phi_result (phi); > > + use_operand_p imm_use; > > + gimple *use_stmt; > > + > > + /* If the PHI's result is never used, then we can just > > + ignore it an. */ > > + if (has_zero_uses (result)) > > + continue; > > + > > + /* Get the single use of the result of this PHI node. */ > > + if (!single_imm_use (result, &imm_use, &use_stmt) > > + || gimple_code (use_stmt) != GIMPLE_PHI > > + || gimple_bb (use_stmt) != dest > > + || gimple_phi_arg_def (use_stmt, dest_idx) != result) > > + return false; > > + } > > + } > > + > > if (current_loops) > > { > > /* Protect loop headers. */ > > @@ -1247,6 +1284,14 @@ remove_forwarder_block_with_phi (basic_block bb) > > basic_block dest = succ->dest; > > basic_block dombb, domdest, dom; > > > > + /* We have to feed into another basic block with PHI > > + nodes. */ > > + if (gimple_seq_empty_p (phi_nodes (dest)) > > + /* We don't want to deal with a basic block with > > + abnormal edges. */ > > + || bb_has_abnormal_pred (bb)) > > + return false; > > + > > /* Record BB's single pred in case we need to update the father > > loop's latch information later. */ > > basic_block pred = NULL; > > @@ -1429,65 +1474,14 @@ pass_merge_phi::execute (function *fun) > > unsigned n = last_basic_block_for_fn (fun); > > for (unsigned i = NUM_FIXED_BLOCKS; i < n; i++) > > { > > - basic_block dest; > > basic_block bb = BASIC_BLOCK_FOR_FN (fun, i); > > if (!bb) > > continue; > > > > /* Look for a forwarder block with PHI nodes. */ > > - if (!tree_forwarder_block_p (bb, true)) > > - continue; > > - > > - dest = single_succ (bb); > > - > > - /* We have to feed into another basic block with PHI > > - nodes. */ > > - if (gimple_seq_empty_p (phi_nodes (dest)) > > - /* We don't want to deal with a basic block with > > - abnormal edges. */ > > - || bb_has_abnormal_pred (bb)) > > - continue; > > - > > - /* If BB 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 (dominated_by_p (CDI_DOMINATORS, dest, bb)) > > - { > > - gphi_iterator gsi; > > - unsigned int dest_idx = single_succ_edge (bb)->dest_idx; > > - > > - /* BB dominates DEST. There may be many users of the PHI > > - nodes in BB. However, there is still a trivial case we > > - can handle. If the result of every PHI in BB is used > > - only by a PHI in DEST, then we can trivially merge the > > - PHI nodes from BB into DEST. */ > > - for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); > > - gsi_next (&gsi)) > > - { > > - gphi *phi = gsi.phi (); > > - tree result = gimple_phi_result (phi); > > - use_operand_p imm_use; > > - gimple *use_stmt; > > - > > - /* If the PHI's result is never used, then we can just > > - ignore it. */ > > - if (has_zero_uses (result)) > > - continue; > > - > > - /* Get the single use of the result of this PHI node. */ > > - if (!single_imm_use (result, &imm_use, &use_stmt) > > - || gimple_code (use_stmt) != GIMPLE_PHI > > - || gimple_bb (use_stmt) != dest > > - || gimple_phi_arg_def (use_stmt, dest_idx) != result) > > - break; > > - } > > - > > - /* If the loop above iterated through all the PHI nodes > > - in BB, then we can merge the PHIs from BB into DEST. */ > > - if (!gsi_end_p (gsi)) > > - continue; > > - } > > - changed |= remove_forwarder_block_with_phi (bb); > > + if (tree_forwarder_block_p (bb, true) > > + && remove_forwarder_block_with_phi (bb)) > > + changed = true; > > } > > > > /* Removing forwarder blocks can cause formerly irreducible loops > > -- > > 2.43.0 > >
