From: Richard Biener <richard.guenther@gmail.com>
To: Andrew Pinski <apinski@marvell.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 1/2] Move copy_phi_arg_into_existing_phi to common location and use it
Date: Thu, 4 May 2023 11:42:56 +0200 [thread overview]
Message-ID: <CAFiYyc3OhNV_AUOve7eYUeLXWLRAXhAiU3wY8Nkm_cgnwbHE2A@mail.gmail.com> (raw)
In-Reply-To: <20230503231224.931656-1-apinski@marvell.com>
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
>
prev parent reply other threads:[~2023-05-04 9:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-03 23:12 Andrew Pinski
2023-05-03 23:12 ` [PATCHv2 2/2] PHIOPT: Improve replace_phi_edge_with_variable for diamond shapped bb Andrew Pinski
2023-05-04 9:43 ` Richard Biener
2023-05-04 9:42 ` Richard Biener [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFiYyc3OhNV_AUOve7eYUeLXWLRAXhAiU3wY8Nkm_cgnwbHE2A@mail.gmail.com \
--to=richard.guenther@gmail.com \
--cc=apinski@marvell.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).