public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, PR 57539] Fix refdesc remapping during inlining
@ 2013-06-12 11:59 Martin Jambor
  2013-06-12 12:13 ` [PATCH, PR 57539] Testcase produced by multidelta and indent Martin Jambor
  2013-06-20 11:28 ` [PATCH, PR 57539] Fix refdesc remapping during inlining Jan Hubicka
  0 siblings, 2 replies; 4+ messages in thread
From: Martin Jambor @ 2013-06-12 11:59 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jan Hubicka

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.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, PR 57539] Testcase produced by multidelta and indent
  2013-06-12 11:59 [PATCH, PR 57539] Fix refdesc remapping during inlining Martin Jambor
@ 2013-06-12 12:13 ` Martin Jambor
  2013-08-06 17:49   ` Martin Jambor
  2013-06-20 11:28 ` [PATCH, PR 57539] Fix refdesc remapping during inlining Jan Hubicka
  1 sibling, 1 reply; 4+ messages in thread
From: Martin Jambor @ 2013-06-12 12:13 UTC (permalink / raw)
  To: GCC Patches, Jan Hubicka

Hi,

On Wed, Jun 12, 2013 at 01:59:29PM +0200, Martin Jambor wrote:
> 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.

Creating a testcase for this bug from scratch is not particularly easy
or reliable because it requires inliner to build up a particular data
structure (and then inline it again), which depends on inlining order
which can easily change.  So I did not want to spend too much time on
it.

However, by running multidelta (several times), indent, multidelta and
indent again on the testcase supplied by the reporter I quickly came
up with the following beast.  The downside is that I have very little
understanding what the testcase does with all potential problems in
the future.  Thus I'm not sure if we want such a testcase in the
testsuite at all.  Nevertheless, it currently tests for the bug, does
not warn (without any -W options) and I'll be fine with reverting it
should any problems arise.

So, is it also OK for the trunk?

Thanks,

Martin


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

testsuite/
	PR tree-optimization/57539
	* gcc.dg/ipa/pr57539.c: New test.

Index: src/gcc/testsuite/gcc.dg/ipa/pr57539.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/ipa/pr57539.c
@@ -0,0 +1,218 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+typedef long unsigned int size_t;
+typedef struct
+{
+}
+box;
+typedef struct
+{
+}
+textpara_t;
+typedef struct _dtlink_s Dtlink_t;
+typedef struct _dtdisc_s Dtdisc_t;
+typedef struct _dtmethod_s Dtmethod_t;
+typedef struct _dt_s Dt_t;
+typedef void *(*Dtmemory_f) (Dt_t *, void *, size_t, Dtdisc_t *);
+typedef void *(*Dtsearch_f) (Dt_t *, void *, int);
+typedef void *(*Dtmake_f) (Dt_t *, void *, Dtdisc_t *);
+typedef void (*Dtfree_f) (Dt_t *, void *, Dtdisc_t *);
+typedef int (*Dtcompar_f) (Dt_t *, void *, void *, Dtdisc_t *);
+typedef unsigned int (*Dthash_f) (Dt_t *, void *, Dtdisc_t *);
+typedef int (*Dtevent_f) (Dt_t *, int, void *, Dtdisc_t *);
+struct _dtlink_s
+{
+  Dtlink_t *right;
+};
+struct _dtdisc_s
+{
+  int key;
+  int size;
+  int link;
+  Dtmake_f makef;
+  Dtfree_f freef;
+  Dtcompar_f comparf;
+  Dthash_f hashf;
+  Dtmemory_f memoryf;
+  Dtevent_f eventf;
+};
+struct _dt_s
+{
+  Dtsearch_f searchf;
+};
+extern Dtmethod_t *Dtobag;
+extern Dt_t *dtopen (Dtdisc_t *, Dtmethod_t *);
+extern Dtlink_t *dtflatten (Dt_t *);
+typedef struct Agobj_s Agobj_t;
+typedef struct Agraph_s Agraph_t;
+typedef struct Agnode_s Agnode_t;
+typedef struct Agedge_s Agedge_t;
+typedef struct Agdesc_s Agdesc_t;
+typedef struct Agdisc_s Agdisc_t;
+typedef struct Agrec_s Agrec_t;
+struct Agobj_s
+{
+  Agrec_t *data;
+};
+struct Agdesc_s
+{
+};
+extern Agraph_t *agopen (char *name, Agdesc_t desc, Agdisc_t * disc);
+extern Agnode_t *agfstnode (Agraph_t * g);
+extern Agnode_t *agnxtnode (Agraph_t * g, Agnode_t * n);
+extern Agedge_t *agedge (Agraph_t * g, Agnode_t * t, Agnode_t * h, char *name,
+			 int createflag);
+extern Agedge_t *agfstout (Agraph_t * g, Agnode_t * n);
+extern Agedge_t *agnxtout (Agraph_t * g, Agedge_t * e);
+extern Agdesc_t Agdirected, Agstrictdirected, Agundirected,
+  Agstrictundirected;
+typedef struct Agraph_s graph_t;
+typedef struct Agnode_s node_t;
+typedef struct Agedge_s edge_t;
+typedef union inside_t
+{
+  unsigned short minlen;
+}
+Agedgeinfo_t;
+extern void *gmalloc (size_t);
+typedef enum
+{ AM_NONE, AM_VOR, AM_SCALE, AM_NSCALE, AM_SCALEXY, AM_PUSH, AM_PUSHPULL,
+    AM_ORTHO, AM_ORTHO_YX, AM_ORTHOXY, AM_ORTHOYX, AM_PORTHO, AM_PORTHO_YX,
+    AM_PORTHOXY, AM_PORTHOYX, AM_COMPRESS, AM_VPSC, AM_IPSEP, AM_PRISM }
+adjust_mode;
+typedef struct nitem
+{
+  Dtlink_t link;
+  int val;
+  node_t *cnode;
+  box bb;
+}
+nitem;
+typedef int (*distfn) (box *, box *);
+typedef int (*intersectfn) (nitem *, nitem *);
+static int
+cmpitem (Dt_t * d, int *p1, int *p2, Dtdisc_t * disc)
+{
+}
+static Dtdisc_t constr =
+  { __builtin_offsetof (nitem, val), sizeof (int), __builtin_offsetof (nitem,
+								       link),
+((Dtmake_f) 0), ((Dtfree_f) 0), (Dtcompar_f) cmpitem, ((Dthash_f) 0), ((Dtmemory_f) 0),
+((Dtevent_f) 0) };
+static int
+distX (box * b1, box * b2)
+{
+}
+
+static int
+intersectY0 (nitem * p, nitem * q)
+{
+}
+
+static int
+intersectY (nitem * p, nitem * q)
+{
+}
+
+static void
+mapGraphs (graph_t * g, graph_t * cg, distfn dist)
+{
+  node_t *n;
+  edge_t *e;
+  edge_t *ce;
+  node_t *t;
+  node_t *h;
+  nitem *tp;
+  nitem *hp;
+  int delta;
+  for (n = agfstnode (g); n; n = agnxtnode (g, n))
+    {
+      for (e = agfstout (g, n); e; e = agnxtout (g, e))
+	{
+	  delta = dist (&tp->bb, &hp->bb);
+	  ce = agedge (cg, t, h, ((void *) 0), 1);
+	  if ((((Agedgeinfo_t *) (((Agobj_t *) (ce))->data))->minlen) < delta)
+	    {
+	      if ((((Agedgeinfo_t *) (((Agobj_t *) (ce))->data))->minlen) ==
+		  0.0)
+		{
+		}
+	    }
+	}
+    }
+}
+
+static graph_t *
+mkNConstraintG (graph_t * g, Dt_t * list, intersectfn intersect, distfn dist)
+{
+  nitem *p;
+  nitem *nxp;
+  edge_t *e;
+  graph_t *cg = agopen ("cg", Agstrictdirected, ((Agdisc_t *) 0));
+  for (p = (nitem *) dtflatten (list); p;
+       p = (nitem *) (((Dtlink_t *) ((Dtlink_t *) p))->right))
+    {
+      for (nxp = (nitem *) (((Dtlink_t *) ((Dtlink_t *) p))->right); nxp;
+	   nxp = (nitem *) (((Dtlink_t *) ((Dtlink_t *) nxp))->right))
+	{
+	  if (intersect (p, nxp))
+	    {
+	      e = agedge (cg, p->cnode, nxp->cnode, ((void *) 0), 1);
+	    }
+  }} for (p = (nitem *) dtflatten (list); p;
+	    p = (nitem *) (((Dtlink_t *) ((Dtlink_t *) p))->right))
+    {
+    }
+}
+
+static graph_t *
+mkConstraintG (graph_t * g, Dt_t * list, intersectfn intersect, distfn dist)
+{
+  graph_t *vg;
+  graph_t *cg = agopen ("cg", Agstrictdirected, ((Agdisc_t *) 0));
+  mapGraphs (vg, cg, dist);
+}
+
+static void
+constrainX (graph_t * g, nitem * nlist, int nnodes, intersectfn ifn,
+	    int ortho)
+{
+  Dt_t *list = dtopen (&constr, Dtobag);
+  nitem *p = nlist;
+  graph_t *cg;
+  int i;
+  for (i = 0; i < nnodes; i++)
+    {
+      (*(((Dt_t *) (list))->searchf)) ((list), (void *) (p), 0000001);
+      p++;
+  } if (ortho)
+    cg = mkConstraintG (g, list, ifn, distX);
+  else
+    cg = mkNConstraintG (g, list, ifn, distX);
+}
+
+int
+cAdjust (graph_t * g, int mode)
+{
+  int ret, i, nnodes = agnnodes (g);
+  nitem *nlist = (nitem *) gmalloc ((nnodes) * sizeof (nitem));
+  node_t *n;
+  for (n = agfstnode (g); n; n = agnxtnode (g, n))
+    {
+    }
+  if (overlaps (nlist, nnodes))
+    {
+      switch ((adjust_mode) mode)
+	{
+	case AM_ORTHOXY:
+	  constrainX (g, nlist, nnodes, intersectY, 1);
+	case AM_ORTHO:
+	  constrainX (g, nlist, nnodes, intersectY0, 1);
+	  constrainX (g, nlist, nnodes, intersectY, 1);
+	case AM_PORTHO:
+	default:
+	  constrainX (g, nlist, nnodes, intersectY0, 0);
+	}
+    }
+}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, PR 57539] Fix refdesc remapping during inlining
  2013-06-12 11:59 [PATCH, PR 57539] Fix refdesc remapping during inlining Martin Jambor
  2013-06-12 12:13 ` [PATCH, PR 57539] Testcase produced by multidelta and indent Martin Jambor
@ 2013-06-20 11:28 ` Jan Hubicka
  1 sibling, 0 replies; 4+ messages in thread
From: Jan Hubicka @ 2013-06-20 11:28 UTC (permalink / raw)
  To: GCC Patches, Jan Hubicka

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, PR 57539] Testcase produced by multidelta and indent
  2013-06-12 12:13 ` [PATCH, PR 57539] Testcase produced by multidelta and indent Martin Jambor
@ 2013-08-06 17:49   ` Martin Jambor
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Jambor @ 2013-08-06 17:49 UTC (permalink / raw)
  To: GCC Patches

Hi,

nobody replied to this, presumably because nobody has any objections
but also nobody really cares, and the patch sits there in my queue.
OTOH, some time ago people on IRC told me to just commit it.  It only
adds a testcase so I'll take the risk of being yelled at and go ahead.
I believe there will be no problems but of course I'll revert it
immediately if any occur (more information why trouble may happen
below in the original mail).

Martin


On Wed, Jun 12, 2013 at 02:13:45PM +0200, Martin Jambor wrote:
> Hi,
> 
> On Wed, Jun 12, 2013 at 01:59:29PM +0200, Martin Jambor wrote:
> > 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.
> 
> Creating a testcase for this bug from scratch is not particularly easy
> or reliable because it requires inliner to build up a particular data
> structure (and then inline it again), which depends on inlining order
> which can easily change.  So I did not want to spend too much time on
> it.
> 
> However, by running multidelta (several times), indent, multidelta and
> indent again on the testcase supplied by the reporter I quickly came
> up with the following beast.  The downside is that I have very little
> understanding what the testcase does with all potential problems in
> the future.  Thus I'm not sure if we want such a testcase in the
> testsuite at all.  Nevertheless, it currently tests for the bug, does
> not warn (without any -W options) and I'll be fine with reverting it
> should any problems arise.
> 
> So, is it also OK for the trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2013-06-11  Martin Jambor  <mjambor@suse.cz>
> 
> testsuite/
> 	PR tree-optimization/57539
> 	* gcc.dg/ipa/pr57539.c: New test.
> 
> Index: src/gcc/testsuite/gcc.dg/ipa/pr57539.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/pr57539.c
> @@ -0,0 +1,218 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +typedef long unsigned int size_t;
> +typedef struct
> +{
> +}
> +box;
> +typedef struct
> +{
> +}
> +textpara_t;
> +typedef struct _dtlink_s Dtlink_t;
> +typedef struct _dtdisc_s Dtdisc_t;
> +typedef struct _dtmethod_s Dtmethod_t;
> +typedef struct _dt_s Dt_t;
> +typedef void *(*Dtmemory_f) (Dt_t *, void *, size_t, Dtdisc_t *);
> +typedef void *(*Dtsearch_f) (Dt_t *, void *, int);
> +typedef void *(*Dtmake_f) (Dt_t *, void *, Dtdisc_t *);
> +typedef void (*Dtfree_f) (Dt_t *, void *, Dtdisc_t *);
> +typedef int (*Dtcompar_f) (Dt_t *, void *, void *, Dtdisc_t *);
> +typedef unsigned int (*Dthash_f) (Dt_t *, void *, Dtdisc_t *);
> +typedef int (*Dtevent_f) (Dt_t *, int, void *, Dtdisc_t *);
> +struct _dtlink_s
> +{
> +  Dtlink_t *right;
> +};
> +struct _dtdisc_s
> +{
> +  int key;
> +  int size;
> +  int link;
> +  Dtmake_f makef;
> +  Dtfree_f freef;
> +  Dtcompar_f comparf;
> +  Dthash_f hashf;
> +  Dtmemory_f memoryf;
> +  Dtevent_f eventf;
> +};
> +struct _dt_s
> +{
> +  Dtsearch_f searchf;
> +};
> +extern Dtmethod_t *Dtobag;
> +extern Dt_t *dtopen (Dtdisc_t *, Dtmethod_t *);
> +extern Dtlink_t *dtflatten (Dt_t *);
> +typedef struct Agobj_s Agobj_t;
> +typedef struct Agraph_s Agraph_t;
> +typedef struct Agnode_s Agnode_t;
> +typedef struct Agedge_s Agedge_t;
> +typedef struct Agdesc_s Agdesc_t;
> +typedef struct Agdisc_s Agdisc_t;
> +typedef struct Agrec_s Agrec_t;
> +struct Agobj_s
> +{
> +  Agrec_t *data;
> +};
> +struct Agdesc_s
> +{
> +};
> +extern Agraph_t *agopen (char *name, Agdesc_t desc, Agdisc_t * disc);
> +extern Agnode_t *agfstnode (Agraph_t * g);
> +extern Agnode_t *agnxtnode (Agraph_t * g, Agnode_t * n);
> +extern Agedge_t *agedge (Agraph_t * g, Agnode_t * t, Agnode_t * h, char *name,
> +			 int createflag);
> +extern Agedge_t *agfstout (Agraph_t * g, Agnode_t * n);
> +extern Agedge_t *agnxtout (Agraph_t * g, Agedge_t * e);
> +extern Agdesc_t Agdirected, Agstrictdirected, Agundirected,
> +  Agstrictundirected;
> +typedef struct Agraph_s graph_t;
> +typedef struct Agnode_s node_t;
> +typedef struct Agedge_s edge_t;
> +typedef union inside_t
> +{
> +  unsigned short minlen;
> +}
> +Agedgeinfo_t;
> +extern void *gmalloc (size_t);
> +typedef enum
> +{ AM_NONE, AM_VOR, AM_SCALE, AM_NSCALE, AM_SCALEXY, AM_PUSH, AM_PUSHPULL,
> +    AM_ORTHO, AM_ORTHO_YX, AM_ORTHOXY, AM_ORTHOYX, AM_PORTHO, AM_PORTHO_YX,
> +    AM_PORTHOXY, AM_PORTHOYX, AM_COMPRESS, AM_VPSC, AM_IPSEP, AM_PRISM }
> +adjust_mode;
> +typedef struct nitem
> +{
> +  Dtlink_t link;
> +  int val;
> +  node_t *cnode;
> +  box bb;
> +}
> +nitem;
> +typedef int (*distfn) (box *, box *);
> +typedef int (*intersectfn) (nitem *, nitem *);
> +static int
> +cmpitem (Dt_t * d, int *p1, int *p2, Dtdisc_t * disc)
> +{
> +}
> +static Dtdisc_t constr =
> +  { __builtin_offsetof (nitem, val), sizeof (int), __builtin_offsetof (nitem,
> +								       link),
> +((Dtmake_f) 0), ((Dtfree_f) 0), (Dtcompar_f) cmpitem, ((Dthash_f) 0), ((Dtmemory_f) 0),
> +((Dtevent_f) 0) };
> +static int
> +distX (box * b1, box * b2)
> +{
> +}
> +
> +static int
> +intersectY0 (nitem * p, nitem * q)
> +{
> +}
> +
> +static int
> +intersectY (nitem * p, nitem * q)
> +{
> +}
> +
> +static void
> +mapGraphs (graph_t * g, graph_t * cg, distfn dist)
> +{
> +  node_t *n;
> +  edge_t *e;
> +  edge_t *ce;
> +  node_t *t;
> +  node_t *h;
> +  nitem *tp;
> +  nitem *hp;
> +  int delta;
> +  for (n = agfstnode (g); n; n = agnxtnode (g, n))
> +    {
> +      for (e = agfstout (g, n); e; e = agnxtout (g, e))
> +	{
> +	  delta = dist (&tp->bb, &hp->bb);
> +	  ce = agedge (cg, t, h, ((void *) 0), 1);
> +	  if ((((Agedgeinfo_t *) (((Agobj_t *) (ce))->data))->minlen) < delta)
> +	    {
> +	      if ((((Agedgeinfo_t *) (((Agobj_t *) (ce))->data))->minlen) ==
> +		  0.0)
> +		{
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +static graph_t *
> +mkNConstraintG (graph_t * g, Dt_t * list, intersectfn intersect, distfn dist)
> +{
> +  nitem *p;
> +  nitem *nxp;
> +  edge_t *e;
> +  graph_t *cg = agopen ("cg", Agstrictdirected, ((Agdisc_t *) 0));
> +  for (p = (nitem *) dtflatten (list); p;
> +       p = (nitem *) (((Dtlink_t *) ((Dtlink_t *) p))->right))
> +    {
> +      for (nxp = (nitem *) (((Dtlink_t *) ((Dtlink_t *) p))->right); nxp;
> +	   nxp = (nitem *) (((Dtlink_t *) ((Dtlink_t *) nxp))->right))
> +	{
> +	  if (intersect (p, nxp))
> +	    {
> +	      e = agedge (cg, p->cnode, nxp->cnode, ((void *) 0), 1);
> +	    }
> +  }} for (p = (nitem *) dtflatten (list); p;
> +	    p = (nitem *) (((Dtlink_t *) ((Dtlink_t *) p))->right))
> +    {
> +    }
> +}
> +
> +static graph_t *
> +mkConstraintG (graph_t * g, Dt_t * list, intersectfn intersect, distfn dist)
> +{
> +  graph_t *vg;
> +  graph_t *cg = agopen ("cg", Agstrictdirected, ((Agdisc_t *) 0));
> +  mapGraphs (vg, cg, dist);
> +}
> +
> +static void
> +constrainX (graph_t * g, nitem * nlist, int nnodes, intersectfn ifn,
> +	    int ortho)
> +{
> +  Dt_t *list = dtopen (&constr, Dtobag);
> +  nitem *p = nlist;
> +  graph_t *cg;
> +  int i;
> +  for (i = 0; i < nnodes; i++)
> +    {
> +      (*(((Dt_t *) (list))->searchf)) ((list), (void *) (p), 0000001);
> +      p++;
> +  } if (ortho)
> +    cg = mkConstraintG (g, list, ifn, distX);
> +  else
> +    cg = mkNConstraintG (g, list, ifn, distX);
> +}
> +
> +int
> +cAdjust (graph_t * g, int mode)
> +{
> +  int ret, i, nnodes = agnnodes (g);
> +  nitem *nlist = (nitem *) gmalloc ((nnodes) * sizeof (nitem));
> +  node_t *n;
> +  for (n = agfstnode (g); n; n = agnxtnode (g, n))
> +    {
> +    }
> +  if (overlaps (nlist, nnodes))
> +    {
> +      switch ((adjust_mode) mode)
> +	{
> +	case AM_ORTHOXY:
> +	  constrainX (g, nlist, nnodes, intersectY, 1);
> +	case AM_ORTHO:
> +	  constrainX (g, nlist, nnodes, intersectY0, 1);
> +	  constrainX (g, nlist, nnodes, intersectY, 1);
> +	case AM_PORTHO:
> +	default:
> +	  constrainX (g, nlist, nnodes, intersectY0, 0);
> +	}
> +    }
> +}
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-08-06 17:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-12 11:59 [PATCH, PR 57539] Fix refdesc remapping during inlining Martin Jambor
2013-06-12 12:13 ` [PATCH, PR 57539] Testcase produced by multidelta and indent Martin Jambor
2013-08-06 17:49   ` Martin Jambor
2013-06-20 11:28 ` [PATCH, PR 57539] Fix refdesc remapping during inlining Jan Hubicka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).