This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] O(1) PHI argument look-up - Part 1/n
- From: Diego Novillo <dnovillo at redhat dot com>
- To: Kazu Hirata <kazu at cs dot umass dot edu>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 19 Nov 2004 10:26:23 -0500
- Subject: Re: [patch] O(1) PHI argument look-up - Part 1/n
- Organization: Red Hat Canada
- References: <20041118.095455.74724221.kazu@cs.umass.edu>
On Thu, 2004-11-18 at 09:54 -0500, Kazu Hirata wrote:
> * basic-block.h (edge_def): Add dest_idx.
> * cfg.c (unchecked_make_edge): Initialize dest_idx.
> (remove_edge): Simplify the disconnection of an edge from its
> destination.
> (redirect_edge_succ): Likewise.
> * cfghooks.c (verify_flow_info): Check the consistency of
> dest_idx for each edge.
>
Looks mostly OK. A few comments:
> Index: basic-block.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/basic-block.h,v
> retrieving revision 1.227
> diff -c -d -p -r1.227 basic-block.h
> *** basic-block.h 17 Nov 2004 22:05:58 -0000 1.227
> --- basic-block.h 18 Nov 2004 06:49:17 -0000
> *************** struct edge_def GTY(())
> *** 154,159 ****
> --- 154,162 ----
> int probability; /* biased by REG_BR_PROB_BASE */
> gcov_type count; /* Expected number of executions calculated
> in profile.c */
> +
> + /* The index within edge vector dest->preds. */
>
How about 'The index number corresponding to this edge in the edge
vector dest->preds".
> *************** remove_edge (edge e)
> *** 375,394 ****
>
> gcc_assert (found);
>
> ! found = false;
> ! for (ei = ei_start (dest->preds); (tmp = ei_safe_edge (ei)); )
> ! {
> ! if (tmp == e)
> ! {
> ! VEC_unordered_remove (edge, dest->preds, ei.index);
> ! found = true;
> ! break;
> ! }
> ! else
> ! ei_next (&ei);
> ! }
> !
> ! gcc_assert (found);
>
> free_edge (e);
> }
> --- 378,386 ----
>
> gcc_assert (found);
>
> ! VEC_unordered_remove (edge, dest->preds, dest_idx);
> ! if (dest_idx < EDGE_COUNT (dest->preds))
>
Better change this to 'gcc_assert (dest_idx < EDGE_COUNT (dest-
>preds))'. The old code assumed that we should always find the edge.
> redirect_edge_succ (edge e, basic_block new_succ)
> {
> ! edge tmp;
> ! edge_iterator ei;
> ! bool found = false;
> !
> ! /* Disconnect the edge from the old successor block. */
> ! for (ei = ei_start (e->dest->preds); (tmp = ei_safe_edge (ei)); )
> ! {
> ! if (tmp == e)
> ! {
> ! VEC_unordered_remove (edge, e->dest->preds, ei.index);
> ! found = true;
> ! break;
> ! }
> ! else
> ! ei_next (&ei);
> ! }
>
> ! gcc_assert (found);
>
> /* Reconnect the edge to the new successor block. */
> VEC_safe_push (edge, new_succ->preds, e);
> e->dest = new_succ;
> }
>
> /* Like previous but avoid possible duplicate edge. */
> --- 390,406 ----
> void
> redirect_edge_succ (edge e, basic_block new_succ)
> {
> ! basic_block dest = e->dest;
> ! unsigned int dest_idx = e->dest_idx;
>
> ! VEC_unordered_remove (edge, dest->preds, dest_idx);
> ! if (dest_idx < EDGE_COUNT (dest->preds))
>
Same here. The old code assumed we should always find the edge.
OK with those changes.
Diego.