This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Make reference removal and speculative devirtualization coexist peacefully


Hi,

the patch below fixes the first testcase which currently fails to link
because we remove one-too-many references to symbol b and remove it
when creating and inlining speculative call graph edges.  It also
removes the reference when speculation is un-done, transparently in
the call graph edge deletion hook.

Bootstrapped and tested on x86_64-linux, I believe Honza LTO built
Firefox with it and he also approved it in private communication, so
I'll commit it later today.

Thanks,

Martin


2013-09-04  Martin Jambor  <mjambor@suse.cz>

	* ipa-prop.c (remove_described_reference): Accept missing references,
	return false if that hppens, otherwise return true.
	(cgraph_node_for_jfunc): New function.
	(try_decrement_rdesc_refcount): Likewise.
	(try_make_edge_direct_simple_call): Use them.
	(ipa_edge_removal_hook): Remove references from rdescs.
	(ipa_edge_duplication_hook): Clone rdescs and their references
	when the new edge has the same caller as the old one.
	* cgraph.c (cgraph_resolve_speculation): Remove speculative
	reference before removing any edges.

testsuite/
	* g++.dg/ipa/remref-1.C: New test.
	* g++.dg/ipa/remref-2.C: Likewise.

Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -2496,9 +2496,10 @@ ipa_find_agg_cst_for_param (struct ipa_a
 }
 
 /* Remove a reference to SYMBOL from the list of references of a node given by
-   reference description RDESC.  */
+   reference description RDESC.  Return true if the reference has been
+   successfully found and removed.  */
 
-static void
+static bool
 remove_described_reference (symtab_node symbol, struct ipa_cst_ref_desc *rdesc)
 {
   struct ipa_ref *to_del;
@@ -2507,12 +2508,15 @@ remove_described_reference (symtab_node
   origin = rdesc->cs;
   to_del = ipa_find_reference ((symtab_node) origin->caller, symbol,
 			       origin->call_stmt, origin->lto_stmt_uid);
-  gcc_assert (to_del);
+  if (!to_del)
+    return false;
+
   ipa_remove_reference (to_del);
   if (dump_file)
     fprintf (dump_file, "ipa-prop: Removed a reference from %s/%i to %s.\n",
 	     xstrdup (cgraph_node_name (origin->caller)),
 	     origin->caller->symbol.order, xstrdup (symtab_node_name (symbol)));
+  return true;
 }
 
 /* If JFUNC has a reference description with refcount different from
@@ -2529,6 +2533,45 @@ jfunc_rdesc_usable (struct ipa_jump_func
     return NULL;
 }
 
+/* If the value of constant jump function JFUNC is an address of a function
+   declaration, return the associated call graph node.  Otherwise return
+   NULL.  */
+
+static cgraph_node *
+cgraph_node_for_jfunc (struct ipa_jump_func *jfunc)
+{
+  gcc_checking_assert (jfunc->type == IPA_JF_CONST);
+  tree cst = ipa_get_jf_constant (jfunc);
+  if (TREE_CODE (cst) != ADDR_EXPR
+      || TREE_CODE (TREE_OPERAND (cst, 0)) != FUNCTION_DECL)
+    return NULL;
+
+  return cgraph_get_node (TREE_OPERAND (cst, 0));
+}
+
+
+/* If JFUNC is a constant jump function with a usable rdesc, decrement its
+   refcount and if it hits zero, remove reference to SYMBOL from the caller of
+   the edge specified in the rdesc.  Return false if either the symbol or the
+   reference could not be found, otherwise return true.  */
+
+static bool
+try_decrement_rdesc_refcount (struct ipa_jump_func *jfunc)
+{
+  struct ipa_cst_ref_desc *rdesc;
+  if (jfunc->type == IPA_JF_CONST
+      && (rdesc = jfunc_rdesc_usable (jfunc))
+      && --rdesc->refcount == 0)
+    {
+      symtab_node symbol = (symtab_node) cgraph_node_for_jfunc (jfunc);
+      if (!symbol)
+	return false;
+
+      return remove_described_reference (symbol, rdesc);
+    }
+  return true;
+}
+
 /* Try to find a destination for indirect edge IE that corresponds to a simple
    call or a call of a member function pointer and where the destination is a
    pointer formal parameter described by jump function JFUNC.  If it can be
@@ -2544,7 +2587,6 @@ try_make_edge_direct_simple_call (struct
   tree target;
   bool agg_contents = ie->indirect_info->agg_contents;
   bool speculative = ie->speculative;
-  struct ipa_cst_ref_desc *rdesc;
 
   if (ie->indirect_info->agg_contents)
     target = ipa_find_agg_cst_for_param (&jfunc->agg,
@@ -2557,11 +2599,16 @@ try_make_edge_direct_simple_call (struct
   cs = ipa_make_edge_direct_to_target (ie, target);
 
   /* FIXME: speculative edges can be handled.  */
-  if (cs && !agg_contents && !speculative
-      && jfunc->type == IPA_JF_CONST
-      && (rdesc = jfunc_rdesc_usable (jfunc))
-      && --rdesc->refcount == 0)
-    remove_described_reference ((symtab_node) cs->callee, rdesc);
+  if (cs && !agg_contents && !speculative)
+    {
+      bool ok;
+      gcc_checking_assert (cs->callee
+			   && (jfunc->type != IPA_JF_CONST
+			       || !cgraph_node_for_jfunc (jfunc)
+			       || cs->callee == cgraph_node_for_jfunc (jfunc)));
+      ok = try_decrement_rdesc_refcount (jfunc);
+      gcc_checking_assert (ok);
+    }
 
   return cs;
 }
@@ -2817,7 +2864,9 @@ propagate_controlled_uses (struct cgraph
 	      if (n)
 		{
 		  struct cgraph_node *clone;
-		  remove_described_reference ((symtab_node) n, rdesc);
+		  bool ok;
+		  ok = remove_described_reference ((symtab_node) n, rdesc);
+		  gcc_checking_assert (ok);
 
 		  clone = cs->caller;
 		  while (clone->global.inlined_to
@@ -2960,9 +3009,21 @@ ipa_set_node_agg_value_chain (struct cgr
 static void
 ipa_edge_removal_hook (struct cgraph_edge *cs, void *data ATTRIBUTE_UNUSED)
 {
-  /* During IPA-CP updating we can be called on not-yet analyze clones.  */
+  struct ipa_edge_args *args;
+
+  /* During IPA-CP updating we can be called on not-yet analyzed clones.  */
   if (vec_safe_length (ipa_edge_args_vector) <= (unsigned)cs->uid)
     return;
+
+  args = IPA_EDGE_REF (cs);
+  if (args->jump_functions)
+    {
+      struct ipa_jump_func *jf;
+      int i;
+      FOR_EACH_VEC_ELT (*args->jump_functions, i, jf)
+	try_decrement_rdesc_refcount (jf);
+    }
+
   ipa_free_edge_args_substructures (IPA_EDGE_REF (cs));
 }
 
@@ -3007,6 +3068,24 @@ ipa_edge_duplication_hook (struct cgraph
 
 	  if (!src_rdesc)
 	    dst_jf->value.constant.rdesc = NULL;
+	  else if (src->caller == dst->caller)
+	    {
+	      struct ipa_ref *ref;
+	      symtab_node n = (symtab_node) cgraph_node_for_jfunc (src_jf);
+	      gcc_checking_assert (n);
+	      ref = ipa_find_reference ((symtab_node) src->caller, n,
+					src->call_stmt, src->lto_stmt_uid);
+	      gcc_checking_assert (ref);
+	      ipa_clone_ref (ref, (symtab_node) dst->caller, ref->stmt);
+
+	      gcc_checking_assert (ipa_refdesc_pool);
+	      struct ipa_cst_ref_desc *dst_rdesc
+		= (struct ipa_cst_ref_desc *) pool_alloc (ipa_refdesc_pool);
+	      dst_rdesc->cs = dst;
+	      dst_rdesc->refcount = src_rdesc->refcount;
+	      dst_rdesc->next_duplicate = NULL;
+	      dst_jf->value.constant.rdesc = dst_rdesc;
+	    }
 	  else if (src_rdesc->cs == src)
 	    {
 	      struct ipa_cst_ref_desc *dst_rdesc;
Index: src/gcc/testsuite/g++.dg/ipa/remref-1.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/remref-1.C
@@ -0,0 +1,36 @@
+/* Verify that indirect-inlining induced removal of referenes will not remove
+   too many references in presence of speculative devirtualization.  */
+/* { dg-do link } */
+/* { dg-options "-O3 -fno-early-inlining"  } */
+
+class A
+{
+  public:
+  virtual void foo(void (*)(void));
+};
+
+static
+void b(void)
+{
+}
+
+void
+A::foo(void (*back)(void))
+{
+  back();
+}
+
+class A *a;
+
+void __attribute__ ((noinline, noclone))
+allocate_a ()
+{
+  a = new A();
+}
+
+main()
+{
+  allocate_a();
+  for (int i=0; i<10000;i++)
+    a->foo(b);
+}
Index: src/gcc/testsuite/g++.dg/ipa/remref-2.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/remref-2.C
@@ -0,0 +1,37 @@
+/* Verify that we survive creation and deletion of references to facilitate
+   reference removal while also doing (unsuccessful) speculative
+   devirtualization.  */
+/* { dg-do link } */
+/* { dg-options "-O3 -fno-early-inlining"  } */
+
+class A
+{
+  public:
+  virtual void __attribute__ ((noinline)) foo(void (*)(void));
+};
+
+static
+void b(void)
+{
+}
+
+void  __attribute__ ((noinline))
+A::foo(void (*back)(void))
+{
+  back();
+}
+
+class A *a;
+
+void __attribute__ ((noinline, noclone))
+allocate_a ()
+{
+  a = new A();
+}
+
+main()
+{
+  allocate_a();
+  for (int i=0; i<10000;i++)
+    a->foo(b);
+}
Index: src/gcc/cgraph.c
===================================================================
--- src.orig/gcc/cgraph.c
+++ src/gcc/cgraph.c
@@ -1225,13 +1225,13 @@ cgraph_resolve_speculation (struct cgrap
     edge->frequency = CGRAPH_FREQ_MAX;
   edge->speculative = false;
   e2->speculative = false;
+  ipa_remove_reference (ref);
   if (e2->indirect_unknown_callee || e2->inline_failed)
     cgraph_remove_edge (e2);
   else
     cgraph_remove_node_and_inline_clones (e2->callee, NULL);
   if (edge->caller->call_site_hash)
     cgraph_update_edge_in_call_site_hash (edge);
-  ipa_remove_reference (ref);
   return edge;
 }
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]