This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Simplify & refactor a bit of tree-ssa-dom.c


On Thu, Sep 19, 2013 at 2:48 PM, Jeff Law <law@redhat.com> wrote:
>
> I find it amazing to look at code I wrote in the past, the occasional WTF
> always makes it worth it.
>
> The code to manage the temporary expression & const/copies tables in
> tree-ssa-dom.c around jump threading looks overly convoluted.  In particular
> the code reused an existing unwind point for the temporary expression stack
> when threading one outgoing edge of a block.
>
> I can only guess this was in response to that code at one time being much
> more expensive than it is now.  Given how cheap this code is now, handling
> the temporary expression stack in the most obvious way seems much wiser.
>
> The refactoring should also make it easier to get some missing expressions
> into the table without tons of unwanted code duplication.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed on
> the trunk.
>
>
>
> commit f71b6fa579d113c2de9ec0ab921bbd2dcc7be43c
> Author: Jeff Law <law@redhat.com>
> Date:   Thu Sep 19 05:54:23 2013 -0600
>
>            * tree-ssa-dom.c (record_temporary_equivalences): New function
>             split out of dom_opt_dom_walker::after_dom_children.
>             (dom_opt_dom_walker::thread_across_edge): Move common code
>             in here from dom_opt_dom_walker::after_dom_children.
>             (dom_opt_dom_walker::after_dom_children): Corresponding
> simplifictions.
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index be5b1d9..6c5f5d6 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,11 @@
> +2013-09-17  Jeff Law  <law@redhat.com>
> +
> +       * tree-ssa-dom.c (record_temporary_equivalences): New function
> +       split out of dom_opt_dom_walker::after_dom_children.
> +       (dom_opt_dom_walker::thread_across_edge): Move common code
> +       in here from dom_opt_dom_walker::after_dom_children.
> +       (dom_opt_dom_walker::after_dom_children): Corresponding
> simplifictions.
> +
>  2013-09-19  Jan Hubicka  <jh@suse.cz>
>
>         * cgraph.c (cgraph_create_edge_1): Avoid uninitialized read
> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
> index aac7aa4..f561386 100644
> --- a/gcc/tree-ssa-dom.c
> +++ b/gcc/tree-ssa-dom.c
> @@ -1070,6 +1070,31 @@ simplify_stmt_for_jump_threading (gimple stmt,
>    return lookup_avail_expr (stmt, false);
>  }
>
> +static void
> +record_temporary_equivalences (edge e)
> +{

This new function lacks a comment.

Richard.

> +  int i;
> +  struct edge_info *edge_info = (struct edge_info *) e->aux;
> +
> +  /* If we have info associated with this edge, record it into
> +     our equivalence tables.  */
> +  if (edge_info)
> +    {
> +      cond_equivalence *eq;
> +      tree lhs = edge_info->lhs;
> +      tree rhs = edge_info->rhs;
> +
> +      /* If we have a simple NAME = VALUE equivalence, record it.  */
> +      if (lhs && TREE_CODE (lhs) == SSA_NAME)
> +       record_const_or_copy (lhs, rhs);
> +
> +      /* If we have 0 = COND or 1 = COND equivalences, record them
> +        into our expression hash tables.  */
> +      for (i = 0; edge_info->cond_equivalences.iterate (i, &eq); ++i)
> +       record_cond (eq);
> +    }
> +}
> +
>  /* Wrapper for common code to attempt to thread an edge.  For example,
>     it handles lazily building the dummy condition and the bookkeeping
>     when jump threading is successful.  */
> @@ -1083,9 +1108,27 @@ dom_opt_dom_walker::thread_across_edge (edge e)
>                             integer_zero_node, integer_zero_node,
>                             NULL, NULL);
>
> +  /* Push a marker on both stacks so we can unwind the tables back to their
> +     current state.  */
> +  avail_exprs_stack.safe_push (NULL);
> +  const_and_copies_stack.safe_push (NULL_TREE);
> +
> +  /* Traversing E may result in equivalences we can utilize.  */
> +  record_temporary_equivalences (e);
> +
> +  /* With all the edge equivalences in the tables, go ahead and attempt
> +     to thread through E->dest.  */
>    ::thread_across_edge (dummy_cond_, e, false,
>                         &const_and_copies_stack,
>                         simplify_stmt_for_jump_threading);
> +
> +  /* And restore the various tables to their state before
> +     we threaded this edge.
> +
> +     XXX The code in tree-ssa-threadedge.c will restore the state of
> +     the const_and_copies table.  We we just have to restore the expression
> +     table.  */
> +  remove_local_expressions_from_table ();
>  }
>
>  /* PHI nodes can create equivalences too.
> @@ -1905,9 +1948,6 @@ dom_opt_dom_walker::after_dom_children (basic_block
> bb)
>        && (single_succ_edge (bb)->flags & EDGE_ABNORMAL) == 0
>        && potentially_threadable_block (single_succ (bb)))
>      {
> -      /* Push a marker on the stack, which thread_across_edge expects
> -        and will remove.  */
> -      const_and_copies_stack.safe_push (NULL_TREE);
>        thread_across_edge (single_succ_edge (bb));
>      }
>    else if ((last = last_stmt (bb))
> @@ -1923,79 +1963,15 @@ dom_opt_dom_walker::after_dom_children (basic_block
> bb)
>        /* Only try to thread the edge if it reaches a target block with
>          more than one predecessor and more than one successor.  */
>        if (potentially_threadable_block (true_edge->dest))
> -       {
> -         struct edge_info *edge_info;
> -         unsigned int i;
> -
> -         /* Push a marker onto the available expression stack so that we
> -            unwind any expressions related to the TRUE arm before
> processing
> -            the false arm below.  */
> -          avail_exprs_stack.safe_push (NULL);
> -         const_and_copies_stack.safe_push (NULL_TREE);
> -
> -         edge_info = (struct edge_info *) true_edge->aux;
> -
> -         /* If we have info associated with this edge, record it into
> -            our equivalence tables.  */
> -         if (edge_info)
> -           {
> -             cond_equivalence *eq;
> -             tree lhs = edge_info->lhs;
> -             tree rhs = edge_info->rhs;
> -
> -             /* If we have a simple NAME = VALUE equivalence, record it.
> */
> -             if (lhs && TREE_CODE (lhs) == SSA_NAME)
> -               record_const_or_copy (lhs, rhs);
> -
> -             /* If we have 0 = COND or 1 = COND equivalences, record them
> -                into our expression hash tables.  */
> -             for (i = 0; edge_info->cond_equivalences.iterate (i, &eq);
> ++i)
> -               record_cond (eq);
> -           }
> -
> -         thread_across_edge (true_edge);
> -
> -         /* And restore the various tables to their state before
> -            we threaded this edge.  */
> -         remove_local_expressions_from_table ();
> -       }
> +       thread_across_edge (true_edge);
>
>        /* Similarly for the ELSE arm.  */
>        if (potentially_threadable_block (false_edge->dest))
> -       {
> -         struct edge_info *edge_info;
> -         unsigned int i;
> -
> -         const_and_copies_stack.safe_push (NULL_TREE);
> -         edge_info = (struct edge_info *) false_edge->aux;
> -
> -         /* If we have info associated with this edge, record it into
> -            our equivalence tables.  */
> -         if (edge_info)
> -           {
> -             cond_equivalence *eq;
> -             tree lhs = edge_info->lhs;
> -             tree rhs = edge_info->rhs;
> -
> -             /* If we have a simple NAME = VALUE equivalence, record it.
> */
> -             if (lhs && TREE_CODE (lhs) == SSA_NAME)
> -               record_const_or_copy (lhs, rhs);
> -
> -             /* If we have 0 = COND or 1 = COND equivalences, record them
> -                into our expression hash tables.  */
> -             for (i = 0; edge_info->cond_equivalences.iterate (i, &eq);
> ++i)
> -               record_cond (eq);
> -           }
> +       thread_across_edge (false_edge);
>
> -         /* Now thread the edge.  */
> -         thread_across_edge (false_edge);
> -
> -         /* No need to remove local expressions from our tables
> -            or restore vars to their original value as that will
> -            be done immediately below.  */
> -       }
>      }
>
> +  /* These remove expressions local to BB from the tables.  */
>    remove_local_expressions_from_table ();
>    restore_vars_to_original_value ();
>  }
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]