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

Message ID c5a607d7084b04596fb973356a71d9d3905b4969.1526551813.git.mliska@suse.cz
State New
Headers show
Series
  • Finish transition of {symbol,call}_summary.
Related show

Commit Message

Martin Liška April 20, 2018, 9:54 a.m.
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(-)

Comments

Jan Hubicka June 7, 2018, 12:16 p.m. | #1
> 

> 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
Martin Liška June 8, 2018, 12:05 p.m. | #2
On 06/07/2018 02:16 PM, Jan Hubicka wrote:
>>

>> 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

> 


Ok, I'm doing that with patch that I attach.

Martin
From f3a2951a3bbfb4091b7d4d141adb14a181eebc0a Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>

Date: Fri, 8 Jun 2018 12:50:18 +0200
Subject: [PATCH] Replace some ::get_create with ::get in IPA inline.

gcc/ChangeLog:

2018-06-08  Martin Liska  <mliska@suse.cz>

	* ipa-inline-analysis.c (simple_edge_hints): Use ::get method.
	* ipa-inline.h (estimate_edge_growth): Likewise.
---
 gcc/ipa-inline-analysis.c | 8 ++++----
 gcc/ipa-inline.h          | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
index 9a7267395ea..c781d368a8a 100644
--- a/gcc/ipa-inline-analysis.c
+++ b/gcc/ipa-inline-analysis.c
@@ -96,10 +96,10 @@ simple_edge_hints (struct cgraph_edge *edge)
   struct cgraph_node *to = (edge->caller->global.inlined_to
 			    ? 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 (to)->scc_no
-	 == ipa_fn_summaries->get_create (callee)->scc_no
-      && !edge->recursive_p ())
+  int to_scc_no = ipa_fn_summaries->get (to)->scc_no;
+  int callee_scc_no = ipa_fn_summaries->get (callee)->scc_no;
+
+  if (to_scc_no && to_scc_no  == callee_scc_no && !edge->recursive_p ())
     hints |= INLINE_HINT_same_scc;
 
   if (callee->lto_file_data && edge->caller->lto_file_data
diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h
index 15825bca820..02d6da06f6d 100644
--- a/gcc/ipa-inline.h
+++ b/gcc/ipa-inline.h
@@ -81,7 +81,7 @@ estimate_edge_size (struct cgraph_edge *edge)
 static inline int
 estimate_edge_growth (struct cgraph_edge *edge)
 {
-  ipa_call_summary *s = ipa_call_summaries->get_create (edge);
+  ipa_call_summary *s = ipa_call_summaries->get (edge);
   gcc_checking_assert (s->call_stmt_size || !edge->callee->analyzed);
   return (estimate_edge_size (edge) - s->call_stmt_size);
 }
-- 
2.17.0

Patch

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);
       scev_finalize ();
     }
   FOR_ALL_BB_FN (bb, my_function)
@@ -2384,8 +2383,9 @@  analyze_function_body (struct cgraph_node *node, bool early)
 	  e->aux = NULL;
 	}
     }
-  ipa_fn_summaries->get_create (node)->time = time;
-  ipa_fn_summaries->get_create (node)->self_size = size;
+  ipa_fn_summary *s = ipa_fn_summaries->get_create (node);
+  s->time = time;
+  s->self_size = size;
   nonconstant_names.release ();
   ipa_release_body_info (&fbi);
   if (opt_for_fn (node->decl, optimize))
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
       && !edge->recursive_p ())
     hints |= INLINE_HINT_same_scc;
diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index b015db07d15..12f5ebfd582 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -1559,10 +1559,10 @@  recursive_inlining (struct cgraph_edge *edge,
     fprintf (dump_file,
 	     "\n   Inlined %i times, "
 	     "body grown from size %i to %i, time %f to %f\n", n,
-	     ipa_fn_summaries->get_create (master_clone)->size,
-	     ipa_fn_summaries->get_create (node)->size,
-	     ipa_fn_summaries->get_create (master_clone)->time.to_double (),
-	     ipa_fn_summaries->get_create (node)->time.to_double ());
+	     ipa_fn_summaries->get (master_clone)->size,
+	     ipa_fn_summaries->get (node)->size,
+	     ipa_fn_summaries->get (master_clone)->time.to_double (),
+	     ipa_fn_summaries->get (node)->time.to_double ());
 
   /* Remove master clone we used for inlining.  We rely that clones inlined
      into master clone gets queued just before master clone so we don't
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);
 }
 
 /* Return estimated callee runtime increase after inlining