diff mbox

Fix some issues with speculative devirtualization machinery

Message ID 20130826113618.GA20835@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Aug. 26, 2013, 11:36 a.m. UTC 
Hi,
by making speculative indirect call machinery to be used by ipa-devirt I put it on
wild on firefox converting about 30% of all polymorphic calls in the progrma (30000).
This has provoked some issues I missed previously this patch fixes:

1) dumping - we no longer do only indirect calls from other units.
2) Adds sanity check to cgraph_speculative_call_info with explanation comment
   since it already confused Maritn (Liska).  I also noticed that direct/indirect
   names are swapped that is very confusing
3) I noticed that when we resolve speculation in a way so direct edge wins
   and the edge is already inline, its frequency increases that ought to rescale
   all callees of the node.  I want to do this as part of edge scaling in
   ipa-inline-transform, but droped a FIXME.
4) cgraph_redirect_edge_call_stmt_to_callee ICEd when especulation was
   wrong and dumping was enabled.
5) ipa-inline needs to reset caches more carefuly because of the effect
   described above: resolving a speculation in the middle of function has
   cascading effect that eventually lead to sanity check failure.

Bootstrapped/regtested x86_64-linux, will commit it shortly.

	* cgraph.c (cgraph_turn_edge_to_speculative): Fix debug output.
	(cgraph_speculative_call_info): Fix parameter order and formating;
	add sanity check.
	(cgraph_resolve_speculation): Add FIXME about scaling profiles.
	(cgraph_redirect_edge_call_stmt_to_callee): Fix ICE in debug dump.
	* ipa-inline.c (heap_edge_removal_hook): Reset node growth cache.
	(resolve_noninline_speculation): Update callee keys, too.
diff mbox

Patch

Index: cgraph.c
===================================================================
--- cgraph.c	(revision 201966)
+++ cgraph.c	(working copy)
@@ -1076,8 +1076,8 @@  cgraph_turn_edge_to_speculative (struct
 
   if (dump_file)
     {
-      fprintf (dump_file, "Indirect call -> direct call from"
-	       " other module %s/%i => %s/%i\n",
+      fprintf (dump_file, "Indirect call -> speculative call"
+	       " %s/%i => %s/%i\n",
 	       xstrdup (cgraph_node_name (n)), n->symbol.order,
 	       xstrdup (cgraph_node_name (n2)), n2->symbol.order);
     }
@@ -1113,8 +1113,8 @@  cgraph_turn_edge_to_speculative (struct
 
 void
 cgraph_speculative_call_info (struct cgraph_edge *e,
-			      struct cgraph_edge *&indirect,
 			      struct cgraph_edge *&direct,
+			      struct cgraph_edge *&indirect,
 			      struct ipa_ref *&reference)
 {
   struct ipa_ref *ref;
@@ -1137,16 +1137,18 @@  cgraph_speculative_call_info (struct cgr
 	}
       else
 	for (e = e->caller->callees; 
-	     e2->call_stmt != e->call_stmt || e2->lto_stmt_uid != e->lto_stmt_uid;
+	     e2->call_stmt != e->call_stmt
+	     || e2->lto_stmt_uid != e->lto_stmt_uid;
 	     e = e->next_callee)
 	  ;
     }
   gcc_assert (e->speculative && e2->speculative);
-  indirect = e;
-  direct = e2;
+  direct = e;
+  indirect = e2;
 
   reference = NULL;
-  for (i = 0; ipa_ref_list_reference_iterate (&e->caller->symbol.ref_list, i, ref); i++)
+  for (i = 0; ipa_ref_list_reference_iterate (&e->caller->symbol.ref_list,
+					      i, ref); i++)
     if (ref->speculative
 	&& ((ref->stmt && ref->stmt == e->call_stmt)
 	    || (ref->lto_stmt_uid == e->lto_stmt_uid)))
@@ -1154,6 +1156,11 @@  cgraph_speculative_call_info (struct cgr
 	reference = ref;
 	break;
       }
+
+  /* Speculative edge always consist of all three components - direct edge,
+     indirect and reference.  */
+  
+  gcc_assert (e && e2 && ref);
 }
 
 /* Redirect callee of E to N.  The function does not update underlying
@@ -1209,6 +1216,8 @@  cgraph_resolve_speculation (struct cgrap
         fprintf (dump_file, "Speculative call turned into direct call.\n");
       edge = e2;
       e2 = tmp;
+      /* FIXME:  If EDGE is inlined, we should scale up the frequencies and counts
+         in the functions inlined through it.  */
     }
   edge->count += e2->count;
   edge->frequency += e2->frequency;
@@ -1305,12 +1314,12 @@  cgraph_redirect_edge_call_stmt_to_callee
       else if (!gimple_check_call_matching_types (e->call_stmt, e->callee->symbol.decl,
 						  true))
 	{
-	  e = cgraph_resolve_speculation (e, NULL);
 	  if (dump_file)
 	    fprintf (dump_file, "Not expanding speculative call of %s/%i -> %s/%i\n"
 		     "Type mismatch.\n",
 		     xstrdup (cgraph_node_name (e->caller)), e->caller->symbol.order,
 		     xstrdup (cgraph_node_name (e->callee)), e->callee->symbol.order);
+	  e = cgraph_resolve_speculation (e, NULL);
 	}
       /* Expand speculation into GIMPLE code.  */
       else
Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 201966)
+++ ipa-inline.c	(working copy)
@@ -1397,6 +1397,8 @@  add_new_edges_to_heap (fibheap_t heap, v
 static void
 heap_edge_removal_hook (struct cgraph_edge *e, void *data)
 {
+  if (e->callee)
+    reset_node_growth_cache (e->callee);
   if (e->aux)
     {
       fibheap_delete_node ((fibheap_t)data, (fibnode_t)e->aux);
@@ -1467,12 +1469,12 @@  resolve_noninline_speculation (fibheap_t
       bitmap updated_nodes = BITMAP_ALLOC (NULL);
 
       cgraph_resolve_speculation (edge, NULL);
-      reset_node_growth_cache (where);
       reset_edge_caches (where);
       inline_update_overall_summary (where);
       update_caller_keys (edge_heap, where,
 			  updated_nodes, NULL);
-      reset_node_growth_cache (where);
+      update_callee_keys (edge_heap, where,
+			  updated_nodes);
       BITMAP_FREE (updated_nodes);
     }
 }