Patchwork [PR,57539] Fix refdesc remapping during inlining

login
register
mail settings
Submitter Martin Jambor
Date June 12, 2013, 11:59 a.m.
Message ID <20130612115928.GB26236@virgil.suse>
Download mbox | patch
Permalink /patch/250745/
State New
Headers show

Comments

Martin Jambor - June 12, 2013, 11:59 a.m.
Hi,

PR 57539 revealed two problems with remapping reference descriptors
during cloning of trees of inlined call graph nodes.  First, when
indirect inlining is involved, we happily remove the reference
descriptor itself by calling ipa_free_edge_args_substructures in
ipa_propagate_indirect_call_infos.  Second, the current remapping code
does not work because the global.inlined_to field of the destination
caller is not yet set.

The patch below fixes the first problem by not calling the freeing
function and the second one by making cgraph_clone_node set the
required field prior to calling any duplication hooks (which is the
only place where we have the pair of corresponding source and
destination edge at our disposal so the duplication/remapping has to
happen there).  I have also shortened the lists of corresponding
references and cleared up the search loop a little.

I'll post a testcase in a separate patch.  This one bootstrapped and
passed testsuite on x86_64-linux without any issues.  OK for trunk?

Thanks,

Martin


2013-06-10  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/57539
	* cgraphclones.c (cgraph_clone_node): Add parameter new_inlined_to, set
	global.inlined_to of the new node to it.  All callers changed.
	* ipa-inline-transform.c (clone_inlined_nodes): New variable
	inlining_into, pass it to cgraph_clone_node.
	* ipa-prop.c (ipa_propagate_indirect_call_infos): Do not call
	ipa_free_edge_args_substructures.
	(ipa_edge_duplication_hook): Only add edges from inlined nodes to
	rdesc linked list.  Do not assert rdesc edges have inlined caller.
	Assert we have found an rdesc in the rdesc list.
Jan Hubicka - June 20, 2013, 11:28 a.m.
> Hi,
> 
> PR 57539 revealed two problems with remapping reference descriptors
> during cloning of trees of inlined call graph nodes.  First, when
> indirect inlining is involved, we happily remove the reference
> descriptor itself by calling ipa_free_edge_args_substructures in
> ipa_propagate_indirect_call_infos.  Second, the current remapping code
> does not work because the global.inlined_to field of the destination
> caller is not yet set.
> 
> The patch below fixes the first problem by not calling the freeing
> function and the second one by making cgraph_clone_node set the
> required field prior to calling any duplication hooks (which is the
> only place where we have the pair of corresponding source and
> destination edge at our disposal so the duplication/remapping has to
> happen there).  I have also shortened the lists of corresponding
> references and cleared up the search loop a little.
> 
> I'll post a testcase in a separate patch.  This one bootstrapped and
> passed testsuite on x86_64-linux without any issues.  OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2013-06-10  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/57539
> 	* cgraphclones.c (cgraph_clone_node): Add parameter new_inlined_to, set
> 	global.inlined_to of the new node to it.  All callers changed.
> 	* ipa-inline-transform.c (clone_inlined_nodes): New variable
> 	inlining_into, pass it to cgraph_clone_node.
> 	* ipa-prop.c (ipa_propagate_indirect_call_infos): Do not call
> 	ipa_free_edge_args_substructures.
> 	(ipa_edge_duplication_hook): Only add edges from inlined nodes to
> 	rdesc linked list.  Do not assert rdesc edges have inlined caller.
> 	Assert we have found an rdesc in the rdesc list.

OK,
thanks
Honza
> 
> Index: src/gcc/cgraph.h
> ===================================================================
> --- src.orig/gcc/cgraph.h
> +++ src/gcc/cgraph.h
> @@ -707,7 +707,7 @@ struct cgraph_edge * cgraph_clone_edge (
>  					unsigned, gcov_type, int, bool);
>  struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type,
>  					int, bool, vec<cgraph_edge_p>,
> -					bool);
> +					bool, struct cgraph_node *);
>  tree clone_function_name (tree decl, const char *);
>  struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node,
>  			                          vec<cgraph_edge_p>,
> Index: src/gcc/cgraphclones.c
> ===================================================================
> --- src.orig/gcc/cgraphclones.c
> +++ src/gcc/cgraphclones.c
> @@ -167,13 +167,19 @@ cgraph_clone_edge (struct cgraph_edge *e
>     function's profile to reflect the fact that part of execution is handled
>     by node.  
>     When CALL_DUPLICATOIN_HOOK is true, the ipa passes are acknowledged about
> -   the new clone. Otherwise the caller is responsible for doing so later.  */
> +   the new clone. Otherwise the caller is responsible for doing so later.
> +
> +   If the new node is being inlined into another one, NEW_INLINED_TO should be
> +   the outline function the new one is (even indirectly) inlined to.  All hooks
> +   will see this in node's global.inlined_to, when invoked.  Can be NULL if the
> +   node is not inlined.  */
>  
>  struct cgraph_node *
>  cgraph_clone_node (struct cgraph_node *n, tree decl, gcov_type count, int freq,
>  		   bool update_original,
>  		   vec<cgraph_edge_p> redirect_callers,
> -		   bool call_duplication_hook)
> +		   bool call_duplication_hook,
> +		   struct cgraph_node *new_inlined_to)
>  {
>    struct cgraph_node *new_node = cgraph_create_empty_node ();
>    struct cgraph_edge *e;
> @@ -195,6 +201,7 @@ cgraph_clone_node (struct cgraph_node *n
>    new_node->symbol.externally_visible = false;
>    new_node->local.local = true;
>    new_node->global = n->global;
> +  new_node->global.inlined_to = new_inlined_to;
>    new_node->rtl = n->rtl;
>    new_node->count = count;
>    new_node->frequency = n->frequency;
> @@ -307,7 +314,7 @@ cgraph_create_virtual_clone (struct cgra
>  
>    new_node = cgraph_clone_node (old_node, new_decl, old_node->count,
>  				CGRAPH_FREQ_BASE, false,
> -				redirect_callers, false);
> +				redirect_callers, false, NULL);
>    /* Update the properties.
>       Make clone visible only within this translation unit.  Make sure
>       that is not weak also.
> Index: src/gcc/ipa-inline-transform.c
> ===================================================================
> --- src.orig/gcc/ipa-inline-transform.c
> +++ src/gcc/ipa-inline-transform.c
> @@ -132,6 +132,13 @@ void
>  clone_inlined_nodes (struct cgraph_edge *e, bool duplicate,
>  		     bool update_original, int *overall_size)
>  {
> +  struct cgraph_node *inlining_into;
> +
> +  if (e->caller->global.inlined_to)
> +    inlining_into = e->caller->global.inlined_to;
> +  else
> +    inlining_into = e->caller;
> +
>    if (duplicate)
>      {
>        /* We may eliminate the need for out-of-line copy to be output.
> @@ -167,18 +174,15 @@ clone_inlined_nodes (struct cgraph_edge
>  	{
>  	  struct cgraph_node *n;
>  	  n = cgraph_clone_node (e->callee, e->callee->symbol.decl,
> -				 e->count, e->frequency,
> -				 update_original, vNULL, true);
> +				 e->count, e->frequency, update_original,
> +				 vNULL, true, inlining_into);
>  	  cgraph_redirect_edge_callee (e, n);
>  	}
>      }
>    else
>      symtab_dissolve_same_comdat_group_list ((symtab_node) e->callee);
>  
> -  if (e->caller->global.inlined_to)
> -    e->callee->global.inlined_to = e->caller->global.inlined_to;
> -  else
> -    e->callee->global.inlined_to = e->caller;
> +  e->callee->global.inlined_to = inlining_into;
>  
>    /* Recursively clone all bodies.  */
>    for (e = e->callee->callees; e; e = e->next_callee)
> Index: src/gcc/ipa-inline.c
> ===================================================================
> --- src.orig/gcc/ipa-inline.c
> +++ src/gcc/ipa-inline.c
> @@ -1315,7 +1315,7 @@ recursive_inlining (struct cgraph_edge *
>  	  /* We need original clone to copy around.  */
>  	  master_clone = cgraph_clone_node (node, node->symbol.decl,
>  					    node->count, CGRAPH_FREQ_BASE,
> -					    false, vNULL, true);
> +					    false, vNULL, true, NULL);
>  	  for (e = master_clone->callees; e; e = e->next_callee)
>  	    if (!e->inline_failed)
>  	      clone_inlined_nodes (e, true, false, NULL);
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -2683,9 +2683,6 @@ ipa_propagate_indirect_call_infos (struc
>    propagate_controlled_uses (cs);
>    changed = propagate_info_to_inlined_callees (cs, cs->callee, new_edges);
>  
> -  /* We do not keep jump functions of inlined edges up to date. Better to free
> -     them so we do not access them accidentally.  */
> -  ipa_free_edge_args_substructures (IPA_EDGE_REF (cs));
>    return changed;
>  }
>  
> @@ -2815,9 +2812,12 @@ ipa_edge_duplication_hook (struct cgraph
>  	      dst_rdesc
>  		= (struct ipa_cst_ref_desc *) pool_alloc (ipa_refdesc_pool);
>  	      dst_rdesc->cs = dst;
> -	      dst_rdesc->next_duplicate = src_rdesc->next_duplicate;
> -	      src_rdesc->next_duplicate = dst_rdesc;
>  	      dst_rdesc->refcount = src_rdesc->refcount;
> +	      if (dst->caller->global.inlined_to)
> +		{
> +		  dst_rdesc->next_duplicate = src_rdesc->next_duplicate;
> +		  src_rdesc->next_duplicate = dst_rdesc;
> +		}
>  	      dst_jf->value.constant.rdesc = dst_rdesc;
>  	    }
>  	  else
> @@ -2832,13 +2832,10 @@ ipa_edge_duplication_hook (struct cgraph
>  	      for (dst_rdesc = src_rdesc->next_duplicate;
>  		   dst_rdesc;
>  		   dst_rdesc = dst_rdesc->next_duplicate)
> -		{
> -		  gcc_assert (dst_rdesc->cs->caller->global.inlined_to);
> -		  if (dst_rdesc->cs->caller->global.inlined_to
> -		      == dst->caller->global.inlined_to)
> -		    break;
> -		}
> -
> +		if (dst_rdesc->cs->caller->global.inlined_to
> +		    == dst->caller->global.inlined_to)
> +		  break;
> +	      gcc_assert (dst_rdesc);
>  	      dst_jf->value.constant.rdesc = dst_rdesc;
>  	    }
>  	}
> Index: src/gcc/lto-cgraph.c
> ===================================================================
> --- src.orig/gcc/lto-cgraph.c
> +++ src/gcc/lto-cgraph.c
> @@ -951,7 +951,7 @@ input_node (struct lto_file_decl_data *f
>      {
>        node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl,
>  				0, CGRAPH_FREQ_BASE, false,
> -				vNULL, false);
> +				vNULL, false, NULL);
>      }
>    else
>      node = cgraph_get_create_node (fn_decl);

Patch

Index: src/gcc/cgraph.h
===================================================================
--- src.orig/gcc/cgraph.h
+++ src/gcc/cgraph.h
@@ -707,7 +707,7 @@  struct cgraph_edge * cgraph_clone_edge (
 					unsigned, gcov_type, int, bool);
 struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type,
 					int, bool, vec<cgraph_edge_p>,
-					bool);
+					bool, struct cgraph_node *);
 tree clone_function_name (tree decl, const char *);
 struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node,
 			                          vec<cgraph_edge_p>,
Index: src/gcc/cgraphclones.c
===================================================================
--- src.orig/gcc/cgraphclones.c
+++ src/gcc/cgraphclones.c
@@ -167,13 +167,19 @@  cgraph_clone_edge (struct cgraph_edge *e
    function's profile to reflect the fact that part of execution is handled
    by node.  
    When CALL_DUPLICATOIN_HOOK is true, the ipa passes are acknowledged about
-   the new clone. Otherwise the caller is responsible for doing so later.  */
+   the new clone. Otherwise the caller is responsible for doing so later.
+
+   If the new node is being inlined into another one, NEW_INLINED_TO should be
+   the outline function the new one is (even indirectly) inlined to.  All hooks
+   will see this in node's global.inlined_to, when invoked.  Can be NULL if the
+   node is not inlined.  */
 
 struct cgraph_node *
 cgraph_clone_node (struct cgraph_node *n, tree decl, gcov_type count, int freq,
 		   bool update_original,
 		   vec<cgraph_edge_p> redirect_callers,
-		   bool call_duplication_hook)
+		   bool call_duplication_hook,
+		   struct cgraph_node *new_inlined_to)
 {
   struct cgraph_node *new_node = cgraph_create_empty_node ();
   struct cgraph_edge *e;
@@ -195,6 +201,7 @@  cgraph_clone_node (struct cgraph_node *n
   new_node->symbol.externally_visible = false;
   new_node->local.local = true;
   new_node->global = n->global;
+  new_node->global.inlined_to = new_inlined_to;
   new_node->rtl = n->rtl;
   new_node->count = count;
   new_node->frequency = n->frequency;
@@ -307,7 +314,7 @@  cgraph_create_virtual_clone (struct cgra
 
   new_node = cgraph_clone_node (old_node, new_decl, old_node->count,
 				CGRAPH_FREQ_BASE, false,
-				redirect_callers, false);
+				redirect_callers, false, NULL);
   /* Update the properties.
      Make clone visible only within this translation unit.  Make sure
      that is not weak also.
Index: src/gcc/ipa-inline-transform.c
===================================================================
--- src.orig/gcc/ipa-inline-transform.c
+++ src/gcc/ipa-inline-transform.c
@@ -132,6 +132,13 @@  void
 clone_inlined_nodes (struct cgraph_edge *e, bool duplicate,
 		     bool update_original, int *overall_size)
 {
+  struct cgraph_node *inlining_into;
+
+  if (e->caller->global.inlined_to)
+    inlining_into = e->caller->global.inlined_to;
+  else
+    inlining_into = e->caller;
+
   if (duplicate)
     {
       /* We may eliminate the need for out-of-line copy to be output.
@@ -167,18 +174,15 @@  clone_inlined_nodes (struct cgraph_edge
 	{
 	  struct cgraph_node *n;
 	  n = cgraph_clone_node (e->callee, e->callee->symbol.decl,
-				 e->count, e->frequency,
-				 update_original, vNULL, true);
+				 e->count, e->frequency, update_original,
+				 vNULL, true, inlining_into);
 	  cgraph_redirect_edge_callee (e, n);
 	}
     }
   else
     symtab_dissolve_same_comdat_group_list ((symtab_node) e->callee);
 
-  if (e->caller->global.inlined_to)
-    e->callee->global.inlined_to = e->caller->global.inlined_to;
-  else
-    e->callee->global.inlined_to = e->caller;
+  e->callee->global.inlined_to = inlining_into;
 
   /* Recursively clone all bodies.  */
   for (e = e->callee->callees; e; e = e->next_callee)
Index: src/gcc/ipa-inline.c
===================================================================
--- src.orig/gcc/ipa-inline.c
+++ src/gcc/ipa-inline.c
@@ -1315,7 +1315,7 @@  recursive_inlining (struct cgraph_edge *
 	  /* We need original clone to copy around.  */
 	  master_clone = cgraph_clone_node (node, node->symbol.decl,
 					    node->count, CGRAPH_FREQ_BASE,
-					    false, vNULL, true);
+					    false, vNULL, true, NULL);
 	  for (e = master_clone->callees; e; e = e->next_callee)
 	    if (!e->inline_failed)
 	      clone_inlined_nodes (e, true, false, NULL);
Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -2683,9 +2683,6 @@  ipa_propagate_indirect_call_infos (struc
   propagate_controlled_uses (cs);
   changed = propagate_info_to_inlined_callees (cs, cs->callee, new_edges);
 
-  /* We do not keep jump functions of inlined edges up to date. Better to free
-     them so we do not access them accidentally.  */
-  ipa_free_edge_args_substructures (IPA_EDGE_REF (cs));
   return changed;
 }
 
@@ -2815,9 +2812,12 @@  ipa_edge_duplication_hook (struct cgraph
 	      dst_rdesc
 		= (struct ipa_cst_ref_desc *) pool_alloc (ipa_refdesc_pool);
 	      dst_rdesc->cs = dst;
-	      dst_rdesc->next_duplicate = src_rdesc->next_duplicate;
-	      src_rdesc->next_duplicate = dst_rdesc;
 	      dst_rdesc->refcount = src_rdesc->refcount;
+	      if (dst->caller->global.inlined_to)
+		{
+		  dst_rdesc->next_duplicate = src_rdesc->next_duplicate;
+		  src_rdesc->next_duplicate = dst_rdesc;
+		}
 	      dst_jf->value.constant.rdesc = dst_rdesc;
 	    }
 	  else
@@ -2832,13 +2832,10 @@  ipa_edge_duplication_hook (struct cgraph
 	      for (dst_rdesc = src_rdesc->next_duplicate;
 		   dst_rdesc;
 		   dst_rdesc = dst_rdesc->next_duplicate)
-		{
-		  gcc_assert (dst_rdesc->cs->caller->global.inlined_to);
-		  if (dst_rdesc->cs->caller->global.inlined_to
-		      == dst->caller->global.inlined_to)
-		    break;
-		}
-
+		if (dst_rdesc->cs->caller->global.inlined_to
+		    == dst->caller->global.inlined_to)
+		  break;
+	      gcc_assert (dst_rdesc);
 	      dst_jf->value.constant.rdesc = dst_rdesc;
 	    }
 	}
Index: src/gcc/lto-cgraph.c
===================================================================
--- src.orig/gcc/lto-cgraph.c
+++ src/gcc/lto-cgraph.c
@@ -951,7 +951,7 @@  input_node (struct lto_file_decl_data *f
     {
       node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl,
 				0, CGRAPH_FREQ_BASE, false,
-				vNULL, false);
+				vNULL, false, NULL);
     }
   else
     node = cgraph_get_create_node (fn_decl);