diff mbox series

Free ipa-prop edge summaries for inline calls

Message ID 20191027081326.v3j23qcswowebegr@kam.mff.cuni.cz
State New
Headers show
Series Free ipa-prop edge summaries for inline calls | expand

Commit Message

Jan Hubicka Oct. 27, 2019, 8:13 a.m. UTC
Hi,
this patch makes ipa-prop to free edge summaries (jump functions) for
calls which has been inlined because they are no longer useful.
The main change is to change IPA_EDGE_REF from get_create to get
and thus we need to watch for missing summaires at some places so
combining -O0 and -O2 code works.

Bootstrapped/regtested x86_64-linux, comitted.

	* ipa-cp.c (propagate_constants_across_call): If args are not available
	just drop everything to varying.
	(find_aggregate_values_for_callers_subset): Watch for missing
	edge summary.
	(find_more_scalar_values_for_callers_subs): Likewise.
	* ipa-prop.c (ipa_compute_jump_functions_for_edge,
	update_jump_functions_after_inlining, propagate_controlled_uses):
	Watch for missing summaries.
	(ipa_propagate_indirect_call_infos): Remove summary after propagation
	is finished.
	(ipa_write_node_info): Watch for missing summaries.
	(ipa_read_edge_info): Create new ref.
	(ipa_edge_args_sum_t): Add remove.
	(IPA_EDGE_REF_GET_CREATE): New macro.
	* ipa-fnsummary.c (evaluate_properties_for_edge): Watch for missing
	edge summary.
	(remap_edge_change_prob): Likewise.

Comments

Jan Hubicka Oct. 27, 2019, 10:27 a.m. UTC | #1
> Hi,
> this patch makes ipa-prop to free edge summaries (jump functions) for
> calls which has been inlined because they are no longer useful.
> The main change is to change IPA_EDGE_REF from get_create to get
> and thus we need to watch for missing summaires at some places so
> combining -O0 and -O2 code works.
> 
> Bootstrapped/regtested x86_64-linux, comitted.
> 
> 	* ipa-cp.c (propagate_constants_across_call): If args are not available
> 	just drop everything to varying.
> 	(find_aggregate_values_for_callers_subset): Watch for missing
> 	edge summary.
> 	(find_more_scalar_values_for_callers_subs): Likewise.
> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge,
> 	update_jump_functions_after_inlining, propagate_controlled_uses):
> 	Watch for missing summaries.
> 	(ipa_propagate_indirect_call_infos): Remove summary after propagation
> 	is finished.
> 	(ipa_write_node_info): Watch for missing summaries.
> 	(ipa_read_edge_info): Create new ref.
> 	(ipa_edge_args_sum_t): Add remove.
> 	(IPA_EDGE_REF_GET_CREATE): New macro.
> 	* ipa-fnsummary.c (evaluate_properties_for_edge): Watch for missing
> 	edge summary.
> 	(remap_edge_change_prob): Likewise.
Sadly this patch breaks lto bootstrap with

0xd8193d ipa_edge_args_sum_t::duplicate(cgraph_edge*, cgraph_edge*, ipa_edge_args*, ipa_edge_args*)
        ../../gcc/ipa-prop.c:3883
0xd8a113 call_summary<ipa_edge_args*>::symtab_duplication(cgraph_edge*, cgraph_edge*, void*)
        ../../gcc/symbol-summary.h:771
0x9ecbc9 symbol_table::call_edge_duplication_hooks(cgraph_edge*, cgraph_edge*)
        ../../gcc/cgraph.c:453
0xa0a57a cgraph_edge::clone(cgraph_node*, gcall*, unsigned int, profile_count, profile_count, bool)
        ../../gcc/cgraphclones.c:141
0xa0b5b9 cgraph_node::create_clone(tree_node*, profile_count, bool, vec<cgraph_edge*, va_heap, vl_ptr>, bool, cgraph_node*, ipa_param_adjustments*, char const*)
        ../../gcc/cgraphclones.c:391
0x1f78f3f clone_inlined_nodes(cgraph_edge*, bool, bool, int*)
        ../../gcc/ipa-inline-transform.c:221
0x1f78ff4 clone_inlined_nodes(cgraph_edge*, bool, bool, int*)
        ../../gcc/ipa-inline-transform.c:236
0x1f79c6c inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap, vl_ptr>*, int*, bool, bool*)
        ../../gcc/ipa-inline-transform.c:479
0x1f6ae28 inline_small_functions
        ../../gcc/ipa-inline.c:2151
0x1f6c8e6 ipa_inline
        ../../gcc/ipa-inline.c:2615
0x1f6d792 execute
        ../../gcc/ipa-inline.c:3023

So I suppose that the reference counting still makes use of the jump
functions at inlined edges (even though it does not make much sense).
This patch removes the code releasing them until this is sorted out.

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 277485)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-10-27  Jan Hubicka  <hubicka@ucw.cz>
+
+	* ipa-prop.c (ipa_propagate_indirect_call_infos): Do not remove
+	jump functions.
+
 2019-10-27  Eric Botcazou <ebotcazou@adacore.com>
 
 	* cgraph.c (cgraph_node::rtl_info): Fix cut&pasto in comment.
Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 277484)
+++ ipa-prop.c	(working copy)
@@ -3706,7 +3706,6 @@ ipa_propagate_indirect_call_infos (struc
 
   propagate_controlled_uses (cs);
   changed = propagate_info_to_inlined_callees (cs, cs->callee, new_edges);
-  ipa_edge_args_sum->remove (cs);
 
   return changed;
 }
Martin Jambor Nov. 12, 2019, 10:07 p.m. UTC | #2
On Sun, Oct 27 2019, Jan Hubicka wrote:
> Hi,
> this patch makes ipa-prop to free edge summaries (jump functions) for
> calls which has been inlined because they are no longer useful.
> The main change is to change IPA_EDGE_REF from get_create to get
> and thus we need to watch for missing summaires at some places so
> combining -O0 and -O2 code works.

So, I never quite liked the IPA_NODE_REF and IPA_EDGE_REF macros.
Perhaps now would be a good time to replace them everywhere with the get
(and get_create) methods of the summary holders?

Since ipa_node_params_sum->get might be a bit too long, perhaps we could
use ipcp_node_sum->get or something similar.  And similarly for edges.

What do you think?

Martin


>
> Bootstrapped/regtested x86_64-linux, comitted.
>
> 	* ipa-cp.c (propagate_constants_across_call): If args are not available
> 	just drop everything to varying.
> 	(find_aggregate_values_for_callers_subset): Watch for missing
> 	edge summary.
> 	(find_more_scalar_values_for_callers_subs): Likewise.
> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge,
> 	update_jump_functions_after_inlining, propagate_controlled_uses):
> 	Watch for missing summaries.
> 	(ipa_propagate_indirect_call_infos): Remove summary after propagation
> 	is finished.
> 	(ipa_write_node_info): Watch for missing summaries.
> 	(ipa_read_edge_info): Create new ref.
> 	(ipa_edge_args_sum_t): Add remove.
> 	(IPA_EDGE_REF_GET_CREATE): New macro.
> 	* ipa-fnsummary.c (evaluate_properties_for_edge): Watch for missing
> 	edge summary.
> 	(remap_edge_change_prob): Likewise.
Martin Liška Nov. 13, 2019, 8:16 a.m. UTC | #3
On 11/12/19 11:07 PM, Martin Jambor wrote:
> Since ipa_node_params_sum->get might be a bit too long, perhaps we could
> use ipcp_node_sum->get or something similar.  And similarly for edges.
> 
> What do you think?

I'm for the suggested change!

Martin
Jan Hubicka Nov. 13, 2019, 8:21 a.m. UTC | #4
> On 11/12/19 11:07 PM, Martin Jambor wrote:
> > Since ipa_node_params_sum->get might be a bit too long, perhaps we could
> > use ipcp_node_sum->get or something similar.  And similarly for edges.
> > 
> > What do you think?
> 
> I'm for the suggested change!

Yes, I like it too (and was about to suggest it).  I ould hold it until
the jump function revamp gets into mainline. Lets discuss proper names
for the summaries tomorrow.  In general I sort of consideer use of
"node" in meaning of function a mistake from time callgraph was just a
callgraph.  Also the summary is not ipcp only, so perhaps something
like ipa_function_params_sum but that is long again.

Honza
> 
> Martin
diff mbox series

Patch

Index: ipa-cp.c
===================================================================
--- ipa-cp.c	(revision 277474)
+++ ipa-cp.c	(working copy)
@@ -2309,10 +2309,17 @@  propagate_constants_across_call (struct
   callee_info = IPA_NODE_REF (callee);
 
   args = IPA_EDGE_REF (cs);
-  args_count = ipa_get_cs_argument_count (args);
   parms_count = ipa_get_param_count (callee_info);
   if (parms_count == 0)
     return false;
+  if (!args)
+    {
+      for (i = 0; i < parms_count; i++)
+	ret |= set_all_contains_variable (ipa_get_parm_lattices (callee_info,
+								 i));
+      return ret;
+    }
+  args_count = ipa_get_cs_argument_count (args);
 
   /* If this call goes through a thunk we must not propagate to the first (0th)
      parameter.  However, we might need to uncover a thunk from below a series
@@ -4066,7 +4073,8 @@  find_more_scalar_values_for_callers_subs
 	  if (IPA_NODE_REF (cs->caller)->node_dead)
 	    continue;
 
-	  if (i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
+	  if (!IPA_EDGE_REF (cs)
+	      || i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
 	      || (i == 0
 		  && call_passes_through_thunk_p (cs)))
 	    {
@@ -4135,7 +4143,8 @@  find_more_contexts_for_caller_subset (cg
 
       FOR_EACH_VEC_ELT (callers, j, cs)
 	{
-	  if (i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs)))
+	  if (!IPA_EDGE_REF (cs)
+	      || i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs)))
 	    return;
 	  ipa_jump_func *jfunc = ipa_get_ith_jump_func (IPA_EDGE_REF (cs),
 							    i);
@@ -4451,6 +4460,11 @@  find_aggregate_values_for_callers_subset
 
   FOR_EACH_VEC_ELT (callers, j, cs)
     {
+      if (!IPA_EDGE_REF (cs))
+	{
+	  count = 0;
+	  break;
+	}
       int c = ipa_get_cs_argument_count (IPA_EDGE_REF (cs));
       if (c < count)
 	count = c;
Index: ipa-fnsummary.c
===================================================================
--- ipa-fnsummary.c	(revision 277474)
+++ ipa-fnsummary.c	(working copy)
@@ -452,6 +452,7 @@  evaluate_properties_for_edge (struct cgr
   class ipa_fn_summary *info = ipa_fn_summaries->get (callee);
   vec<tree> known_vals = vNULL;
   vec<ipa_agg_jump_function_p> known_aggs = vNULL;
+  class ipa_edge_args *args;
 
   if (clause_ptr)
     *clause_ptr = inline_p ? 0 : 1 << predicate::not_inlined_condition;
@@ -462,10 +463,10 @@  evaluate_properties_for_edge (struct cgr
 
   if (ipa_node_params_sum
       && !e->call_stmt_cannot_inline_p
-      && ((clause_ptr && info->conds) || known_vals_ptr || known_contexts_ptr))
+      && ((clause_ptr && info->conds) || known_vals_ptr || known_contexts_ptr)
+      && (args = IPA_EDGE_REF (e)) != NULL)
     {
       class ipa_node_params *caller_parms_info, *callee_pi;
-      class ipa_edge_args *args = IPA_EDGE_REF (e);
       class ipa_call_summary *es = ipa_call_summaries->get (e);
       int i, count = ipa_get_cs_argument_count (args);
 
@@ -3160,6 +3161,8 @@  remap_edge_change_prob (struct cgraph_ed
     {
       int i;
       class ipa_edge_args *args = IPA_EDGE_REF (edge);
+      if (!args)
+	return;
       class ipa_call_summary *es = ipa_call_summaries->get (edge);
       class ipa_call_summary *inlined_es
 	= ipa_call_summaries->get (inlined_edge);
Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 277474)
+++ ipa-prop.c	(working copy)
@@ -1854,7 +1854,7 @@  ipa_compute_jump_functions_for_edge (str
 				     struct cgraph_edge *cs)
 {
   class ipa_node_params *info = IPA_NODE_REF (cs->caller);
-  class ipa_edge_args *args = IPA_EDGE_REF (cs);
+  class ipa_edge_args *args = IPA_EDGE_REF_GET_CREATE (cs);
   gcall *call = cs->call_stmt;
   int n, arg_num = gimple_call_num_args (call);
   bool useful_context = false;
@@ -2652,6 +2652,8 @@  update_jump_functions_after_inlining (st
 {
   class ipa_edge_args *top = IPA_EDGE_REF (cs);
   class ipa_edge_args *args = IPA_EDGE_REF (e);
+  if (!args)
+    return;
   int count = ipa_get_cs_argument_count (args);
   int i;
 
@@ -3575,6 +3577,8 @@  static void
 propagate_controlled_uses (struct cgraph_edge *cs)
 {
   class ipa_edge_args *args = IPA_EDGE_REF (cs);
+  if (!args)
+    return;
   struct cgraph_node *new_root = cs->caller->global.inlined_to
     ? cs->caller->global.inlined_to : cs->caller;
   class ipa_node_params *new_root_info = IPA_NODE_REF (new_root);
@@ -3702,6 +3706,7 @@  ipa_propagate_indirect_call_infos (struc
 
   propagate_controlled_uses (cs);
   changed = propagate_info_to_inlined_callees (cs, cs->callee, new_edges);
+  ipa_edge_args_sum->remove (cs);
 
   return changed;
 }
@@ -4380,6 +4385,12 @@  ipa_write_node_info (struct output_block
     {
       class ipa_edge_args *args = IPA_EDGE_REF (e);
 
+      if (!args)
+	{
+	  streamer_write_uhwi (ob, 0);
+	  continue;
+	}
+
       streamer_write_uhwi (ob,
 			   ipa_get_cs_argument_count (args) * 2
 			   + (args->polymorphic_call_contexts != NULL));
@@ -4393,15 +4404,19 @@  ipa_write_node_info (struct output_block
   for (e = node->indirect_calls; e; e = e->next_callee)
     {
       class ipa_edge_args *args = IPA_EDGE_REF (e);
-
-      streamer_write_uhwi (ob,
-			   ipa_get_cs_argument_count (args) * 2
-  			   + (args->polymorphic_call_contexts != NULL));
-      for (j = 0; j < ipa_get_cs_argument_count (args); j++)
+      if (!args)
+	streamer_write_uhwi (ob, 0);
+      else
 	{
-	  ipa_write_jump_function (ob, ipa_get_ith_jump_func (args, j));
-	  if (args->polymorphic_call_contexts != NULL)
-	    ipa_get_ith_polymorhic_call_context (args, j)->stream_out (ob);
+	  streamer_write_uhwi (ob,
+			       ipa_get_cs_argument_count (args) * 2
+			       + (args->polymorphic_call_contexts != NULL));
+	  for (j = 0; j < ipa_get_cs_argument_count (args); j++)
+	    {
+	      ipa_write_jump_function (ob, ipa_get_ith_jump_func (args, j));
+	      if (args->polymorphic_call_contexts != NULL)
+		ipa_get_ith_polymorhic_call_context (args, j)->stream_out (ob);
+	    }
 	}
       ipa_write_indirect_edge_info (ob, e);
     }
@@ -4422,7 +4437,7 @@  ipa_read_edge_info (class lto_input_bloc
     return;
   if (prevails && e->possibly_call_in_translation_unit_p ())
     {
-      class ipa_edge_args *args = IPA_EDGE_REF (e);
+      class ipa_edge_args *args = IPA_EDGE_REF_GET_CREATE (e);
       vec_safe_grow_cleared (args->jump_functions, count);
       if (contexts_computed)
 	vec_safe_grow_cleared (args->polymorphic_call_contexts, count);
Index: ipa-prop.h
===================================================================
--- ipa-prop.h	(revision 277482)
+++ ipa-prop.h	(working copy)
@@ -640,6 +640,11 @@  class GTY((user)) ipa_edge_args_sum_t :
   ipa_edge_args_sum_t (symbol_table *table, bool ggc)
     : call_summary<ipa_edge_args *> (table, ggc) { }
 
+  void remove (cgraph_edge *edge)
+  {
+    call_summary <ipa_edge_args *>::remove (edge);
+  }
+
   /* Hook that is called by summary when an edge is removed.  */
   virtual void remove (cgraph_edge *cs, ipa_edge_args *args);
   /* Hook that is called by summary when an edge is duplicated.  */
@@ -679,7 +684,8 @@  extern GTY(()) function_summary <ipcp_tr
 /* Return the associated parameter/argument info corresponding to the given
    node/edge.  */
 #define IPA_NODE_REF(NODE) (ipa_node_params_sum->get_create (NODE))
-#define IPA_EDGE_REF(EDGE) (ipa_edge_args_sum->get_create (EDGE))
+#define IPA_EDGE_REF(EDGE) (ipa_edge_args_sum->get (EDGE))
+#define IPA_EDGE_REF_GET_CREATE(EDGE) (ipa_edge_args_sum->get_create (EDGE))
 /* This macro checks validity of index returned by
    ipa_get_param_decl_index function.  */
 #define IS_VALID_JUMP_FUNC_INDEX(I) ((I) != -1)