Re: [PATCH 05/14] Use summaries->get where possible. Small refactoring of multiple calls.

[Jan Hubicka <hubicka at ucw dot cz>]

> 
> gcc/ChangeLog:
> 
> 2018-04-24  Martin Liska  <mliska@suse.cz>
> 
> 	* ipa-fnsummary.c (dump_ipa_call_summary): Use ::get method.
> 	(analyze_function_body): Extract multiple calls of get_create.
> 	* ipa-inline-analysis.c (simple_edge_hints): Likewise.
> 	* ipa-inline.c (recursive_inlining): Use ::get method.
> 	* ipa-inline.h (estimate_edge_growth): Likewise.
> ---
>  gcc/ipa-fnsummary.c       | 14 +++++++-------
>  gcc/ipa-inline-analysis.c |  2 +-
>  gcc/ipa-inline.c          |  8 ++++----
>  gcc/ipa-inline.h          |  7 +++----
>  4 files changed, 15 insertions(+), 16 deletions(-)
> 

> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> index 8a6c5d0b5d8..e40b537bf61 100644
> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -850,7 +850,7 @@ dump_ipa_call_summary (FILE *f, int indent, struct cgraph_node *node,
>  	  }
>        if (!edge->inline_failed)
>  	{
> -	  ipa_fn_summary *s = ipa_fn_summaries->get_create (callee);
> +	  ipa_fn_summary *s = ipa_fn_summaries->get (callee);
>  	  fprintf (f, "%*sStack frame offset %i, callee self size %i,"
>  		   " callee size %i\n",
>  		   indent + 2, "",
> @@ -2363,10 +2363,9 @@ analyze_function_body (struct cgraph_node *node, bool early)
>  	    }
>  	  free (body);
>  	}
> -      set_hint_predicate (&ipa_fn_summaries->get_create (node)->loop_iterations,
> -			  loop_iterations);
> -      set_hint_predicate (&ipa_fn_summaries->get_create (node)->loop_stride,
> -			  loop_stride);
> +      ipa_fn_summary *s = ipa_fn_summaries->get_create (node);
> +      set_hint_predicate (&s->loop_iterations, loop_iterations);
> +      set_hint_predicate (&s->loop_stride, loop_stride);

I think you already have pointer info initialized to ipa_fn_summaries->get (node), so just
replace all uses pf ipa_fn_summaries in this function by that. It seems like not very careful
transition (done pehraps by me :)

We may consider modifying our getters to make them pure for gcc so it will
optimize some of those issues for us.  That would require some code refactoring
as you have internal getter with bool parameter that also creates nodes (and
thus is no longer pure)
> diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
> index c4f904730e6..2e30a6d15ba 100644
> --- a/gcc/ipa-inline-analysis.c
> +++ b/gcc/ipa-inline-analysis.c
> @@ -126,7 +126,7 @@ simple_edge_hints (struct cgraph_edge *edge)
>  			    ? edge->caller->global.inlined_to : edge->caller);
>    struct cgraph_node *callee = edge->callee->ultimate_alias_target ();
>    if (ipa_fn_summaries->get_create (to)->scc_no
> -      && ipa_fn_summaries->get_create (to)->scc_no
> +      && ipa_fn_summaries->get (to)->scc_no
>  	 == ipa_fn_summaries->get_create (callee)->scc_no

Please move ipa_fn_summaries->get_create (to)->scc_no out of the
conditional and store the result, so we don't need to call it multiple times.
I think it would be bug if summaries was not ready here, so it should be ->get
only.
> diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h
> index e8ae206d7b7..06bd38e551e 100644
> --- a/gcc/ipa-inline.h
> +++ b/gcc/ipa-inline.h
> @@ -81,10 +81,9 @@ estimate_edge_size (struct cgraph_edge *edge)
>  static inline int
>  estimate_edge_growth (struct cgraph_edge *edge)
>  {
> -  gcc_checking_assert (ipa_call_summaries->get_create (edge)->call_stmt_size
> -		       || !edge->callee->analyzed);
> -  return (estimate_edge_size (edge)
> -	  - ipa_call_summaries->get_create (edge)->call_stmt_size);
> +  ipa_call_summary *s = ipa_call_summaries->get_create (edge);
> +  gcc_checking_assert (s->call_stmt_size || !edge->callee->analyzed);
> +  return (estimate_edge_size (edge) - s->call_stmt_size);

Also if get_create ever created new summary here, it would not have right sizes,
so plase turn it into get here.

OK with those changes. (and if any of those trap, just add FIXME and we can deal with
it incrementally).

Honza