On Thu, May 4, 2023 at 1:13 AM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > While improving replace_phi_edge_with_variable for the diamond formed bb > case, I need a way to copy phi entries from one edge to another as I am > removing a forwarding bb inbetween. I was pointed out that jump threading > code had copy_phi_arg_into_existing_phi which I can use. > I also noticed that both gimple_duplicate_sese_tail and > remove_forwarder_block have similar code so it makes sense to use that > function > in those two locations too. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
OK. > gcc/ChangeLog: > > * tree-cfg.cc (copy_phi_args): New function > (gimple_duplicate_sese_tail): Use it instead of > doing it inline. > * tree-cfg.h (copy_phi_args): New declaration. > * tree-cfgcleanup.cc (remove_forwarder_block): Use > copy_phi_args instead of implementing it inline. > > gcc/ChangeLog: > > * tree-ssa-threadupdate.cc (copy_phi_arg_into_existing_phi): Move to > ... > * tree-cfg.cc (copy_phi_arg_into_existing_phi): Here and remove > static. > (gimple_duplicate_sese_tail): Use copy_phi_arg_into_existing_phi > instead > of an inline version of it. > * tree-cfgcleanup.cc (remove_forwarder_block): Likewise. > * tree-cfg.h (copy_phi_arg_into_existing_phi): New declaration. > --- > gcc/tree-cfg.cc | 38 +++++++++++++++++++++++++----------- > gcc/tree-cfg.h | 1 + > gcc/tree-cfgcleanup.cc | 12 ++---------- > gcc/tree-ssa-threadupdate.cc | 25 ------------------------ > 4 files changed, 30 insertions(+), 46 deletions(-) > > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc > index 4927fc0a8d9..a94dc3c2eea 100644 > --- a/gcc/tree-cfg.cc > +++ b/gcc/tree-cfg.cc > @@ -6802,6 +6802,32 @@ bb_part_of_region_p (basic_block bb, basic_block* bbs, > unsigned n_region) > return false; > } > > + > +/* For each PHI in BB, copy the argument associated with SRC_E to TGT_E. > + Assuming the argument exists, just does not have a value. */ > + > +void > +copy_phi_arg_into_existing_phi (edge src_e, edge tgt_e) > +{ > + int src_idx = src_e->dest_idx; > + int tgt_idx = tgt_e->dest_idx; > + > + /* Iterate over each PHI in e->dest. */ > + for (gphi_iterator gsi = gsi_start_phis (src_e->dest), > + gsi2 = gsi_start_phis (tgt_e->dest); > + !gsi_end_p (gsi); > + gsi_next (&gsi), gsi_next (&gsi2)) > + { > + gphi *src_phi = gsi.phi (); > + gphi *dest_phi = gsi2.phi (); > + tree val = gimple_phi_arg_def (src_phi, src_idx); > + location_t locus = gimple_phi_arg_location (src_phi, src_idx); > + > + SET_PHI_ARG_DEF (dest_phi, tgt_idx, val); > + gimple_phi_arg_set_location (dest_phi, tgt_idx, locus); > + } > +} > + > /* Duplicates REGION consisting of N_REGION blocks. The new blocks > are stored to REGION_COPY in the same order in that they appear > in REGION, if REGION_COPY is not NULL. ENTRY is the entry to > @@ -6847,9 +6873,6 @@ gimple_duplicate_sese_tail (edge entry, edge exit, > gimple_stmt_iterator gsi; > edge sorig, snew; > basic_block exit_bb; > - gphi_iterator psi; > - gphi *phi; > - tree def; > class loop *target, *aloop, *cloop; > > gcc_assert (EDGE_COUNT (exit->src->succs) == 2); > @@ -6947,14 +6970,7 @@ gimple_duplicate_sese_tail (edge entry, edge exit, > gcc_assert (single_succ_edge (region_copy[i])); > e = redirect_edge_and_branch (single_succ_edge (region_copy[i]), > exit_bb); > PENDING_STMT (e) = NULL; > - for (psi = gsi_start_phis (exit_bb); > - !gsi_end_p (psi); > - gsi_next (&psi)) > - { > - phi = psi.phi (); > - def = PHI_ARG_DEF (phi, nexits[0]->dest_idx); > - add_phi_arg (phi, def, e, gimple_phi_arg_location_from_edge (phi, > e)); > - } > + copy_phi_arg_into_existing_phi (nexits[0], e); > } > e = redirect_edge_and_branch (nexits[1], nexits[0]->dest); > PENDING_STMT (e) = NULL; > diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h > index 9b56a68fe9d..9a1605be305 100644 > --- a/gcc/tree-cfg.h > +++ b/gcc/tree-cfg.h > @@ -113,6 +113,7 @@ extern basic_block gimple_switch_default_bb (function *, > gswitch *); > extern edge gimple_switch_edge (function *, gswitch *, unsigned); > extern edge gimple_switch_default_edge (function *, gswitch *); > extern bool cond_only_block_p (basic_block); > +extern void copy_phi_arg_into_existing_phi (edge, edge); > > /* Return true if the LHS of a call should be removed. */ > > diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc > index 42b25312122..193d87e3278 100644 > --- a/gcc/tree-cfgcleanup.cc > +++ b/gcc/tree-cfgcleanup.cc > @@ -610,17 +610,9 @@ remove_forwarder_block (basic_block bb) > > if (s == e) > { > - /* Create arguments for the phi nodes, since the edge was not > + /* Copy arguments for the phi nodes, since the edge was not > here before. */ > - for (gphi_iterator psi = gsi_start_phis (dest); > - !gsi_end_p (psi); > - gsi_next (&psi)) > - { > - gphi *phi = psi.phi (); > - location_t l = gimple_phi_arg_location_from_edge (phi, succ); > - tree def = gimple_phi_arg_def (phi, succ->dest_idx); > - add_phi_arg (phi, unshare_expr (def), s, l); > - } > + copy_phi_arg_into_existing_phi (succ, s); > } > } > > diff --git a/gcc/tree-ssa-threadupdate.cc b/gcc/tree-ssa-threadupdate.cc > index 33a91063638..d5416b21a78 100644 > --- a/gcc/tree-ssa-threadupdate.cc > +++ b/gcc/tree-ssa-threadupdate.cc > @@ -511,31 +511,6 @@ fwd_jt_path_registry::lookup_redirection_data (edge e, > insert_option insert) > } > } > > -/* Similar to copy_phi_args, except that the PHI arg exists, it just > - does not have a value associated with it. */ > - > -static void > -copy_phi_arg_into_existing_phi (edge src_e, edge tgt_e) > -{ > - int src_idx = src_e->dest_idx; > - int tgt_idx = tgt_e->dest_idx; > - > - /* Iterate over each PHI in e->dest. */ > - for (gphi_iterator gsi = gsi_start_phis (src_e->dest), > - gsi2 = gsi_start_phis (tgt_e->dest); > - !gsi_end_p (gsi); > - gsi_next (&gsi), gsi_next (&gsi2)) > - { > - gphi *src_phi = gsi.phi (); > - gphi *dest_phi = gsi2.phi (); > - tree val = gimple_phi_arg_def (src_phi, src_idx); > - location_t locus = gimple_phi_arg_location (src_phi, src_idx); > - > - SET_PHI_ARG_DEF (dest_phi, tgt_idx, val); > - gimple_phi_arg_set_location (dest_phi, tgt_idx, locus); > - } > -} > - > /* Given ssa_name DEF, backtrack jump threading PATH from node IDX > to see if it has constant value in a flow sensitive manner. Set > LOCUS to location of the constant phi arg and return the value. > -- > 2.31.1 >