Re: Minor fixes to ipa-inline-analysis.c

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Nov 7, 2012 at 10:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> while analyzing c-ray I noticed two issues.  First is that I originally set number
> of size/time entries to 32.  Once we reach this limit we conservatively account
> everything as unconditional.  This limit is not met on relatively simple
> testcases, like ray-sphere.  The reason is that aggregate tracking now adds a lot
> of new conditionals on individual fields. While number of arguments hardly exceeds
> 5, the number of fields passed and used easilly. So there is need to increase
> the bound.
>
> While propagating info about non-constant parameters, we should also work in
> reverse postorder rather than in random order, since we can propagate things
> down across SSA graph.
>
> Bootstrapped/regtested & comitted.
>
> Honza
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog   (revision 193284)
> +++ ChangeLog   (working copy)
> @@ -1,3 +1,12 @@
> +2012-11-07  Jan Hubicka  <jh@suse.cz>
> +
> +       * ipa-inline-analysis.c (true_predicate, single_cond_predicate,
> +       reset_inline_edge_summary): Fix
> +       formatting.
> +       (account_size_time): Bump up the limit on number of size/time entries to
> +       256.
> +       (estimate_function_body_sizes): Work in reverse postorder.
> +
>  2012-11-07  David S. Miller  <davem@davemloft.net>
>
>         PR bootstrap/55211
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c       (revision 193284)
> +++ ipa-inline-analysis.c       (working copy)
> @@ -149,7 +149,7 @@ static inline struct predicate
>  true_predicate (void)
>  {
>    struct predicate p;
> -  p.clause[0]=0;
> +  p.clause[0] = 0;
>    return p;
>  }
>
> @@ -160,8 +160,8 @@ static inline struct predicate
>  single_cond_predicate (int cond)
>  {
>    struct predicate p;
> -  p.clause[0]=1 << cond;
> -  p.clause[1]=0;
> +  p.clause[0] = 1 << cond;
> +  p.clause[1] = 0;
>    return p;
>  }
>
> @@ -692,12 +692,14 @@ account_size_time (struct inline_summary
>         found = true;
>          break;
>        }
> -  if (i == 32)
> +  if (i == 256)

Shouldn't this be a --param?

>      {
>        i = 0;
>        found = true;
>        e = &VEC_index (size_time_entry, summary->entry, 0);
>        gcc_assert (!e->predicate.clause[0]);
> +      if (dump_file && (dump_flags & TDF_DETAILS))
> +       fprintf (dump_file, "\t\tReached limit on number of entries, ignoring the predicate.");
>      }
>    if (dump_file && (dump_flags & TDF_DETAILS) && (time || size))
>      {
> @@ -970,7 +972,7 @@ reset_inline_edge_summary (struct cgraph
>      {
>        struct inline_edge_summary *es = inline_edge_summary (e);
>
> -      es->call_stmt_size = es->call_stmt_time =0;
> +      es->call_stmt_size = es->call_stmt_time = 0;
>        if (es->predicate)
>         pool_free (edge_predicate_pool, es->predicate);
>        es->predicate = NULL;
> @@ -2280,6 +2282,8 @@ estimate_function_body_sizes (struct cgr
>    struct predicate bb_predicate;
>    struct ipa_node_params *parms_info = NULL;
>    VEC (predicate_t, heap) *nonconstant_names = NULL;
> +  int nblocks, n;
> +  int *order;
>
>    info->conds = 0;
>    info->entry = 0;
> @@ -2312,8 +2316,12 @@ estimate_function_body_sizes (struct cgr
>    gcc_assert (my_function && my_function->cfg);
>    if (parms_info)
>      compute_bb_predicates (node, parms_info, info);
> -  FOR_EACH_BB_FN (bb, my_function)
> +  gcc_assert (cfun == my_function);
> +  order = XNEWVEC (int, n_basic_blocks);
> +  nblocks = pre_and_rev_post_order_compute (NULL, order, false);
> +  for (n = 0; n < nblocks; n++)
>      {
> +      bb = BASIC_BLOCK (order[n]);
>        freq = compute_call_stmt_bb_frequency (node->symbol.decl, bb);
>
>        /* TODO: Obviously predicates can be propagated down across CFG.  */
> @@ -2486,6 +2494,7 @@ estimate_function_body_sizes (struct cgr
>    time = (time + CGRAPH_FREQ_BASE / 2) / CGRAPH_FREQ_BASE;
>    if (time > MAX_TIME)
>      time = MAX_TIME;
> +  free (order);
>
>    if (!early && nonconstant_names)
>      {