Add speculative call edges verifier and fix one extra bug

Message ID 20200119154025.GC58691@kam.mff.cuni.cz
State New
Headers show
Series
  • Add speculative call edges verifier and fix one extra bug
Related show

Commit Message

Jan Hubicka Jan. 19, 2020, 3:40 p.m.
Hi,
this patch implements verifier and fixes one bug where speculative calls
produced by ipa-devirt ended up having num_speculative_call_targets = 0
instead of 1.

lto-profilebootstrapped/regtested x86_64-linux, will commit it shortly.

Honza
	PR lto/93318
	* cgraph.c (cgraph_edge::make_speculative): Increase number of
	speculative targets.
	(verify_speculative_call): New function
	(cgraph_node::verify_node): Use it.
	* ipa-profile.c (ipa_profile): Fix formating; do not set number of
	speculations.

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index c442f334fe2..187f6ed30ba 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1076,6 +1076,7 @@  cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count,
   e2->speculative_id = speculative_id;
   e2->target_prob = target_prob;
   e2->in_polymorphic_cdtor = in_polymorphic_cdtor;
+  indirect_info->num_speculative_call_targets++;
   count -= e2->count;
   symtab->call_edge_duplication_hooks (this, e2);
   ref = n->create_reference (n2, IPA_REF_ADDR, call_stmt);
@@ -3148,6 +3149,128 @@  cgraph_edge::verify_corresponds_to_fndecl (tree decl)
 #  pragma GCC diagnostic ignored "-Wformat-diag"
 #endif
 
+/* Verify consistency of speculative call in NODE corresponding to STMT
+   and LTO_STMT_UID.  If INDIRECT is set, assume that it is the indirect
+   edge of call sequence. Return true if error is found.
+
+   This function is called to every component of indirect call (direct edges,
+   indirect edge and refs).  To save duplicated work, do full testing only
+   in that case.  */
+static bool
+verify_speculative_call (struct cgraph_node *node, gimple *stmt,
+			 unsigned int lto_stmt_uid,
+			 struct cgraph_edge *indirect)
+{
+  if (indirect == NULL)
+    {
+      for (indirect = node->indirect_calls; indirect;
+	   indirect = indirect->next_callee)
+	if (indirect->call_stmt == stmt
+	    && indirect->lto_stmt_uid == lto_stmt_uid)
+	  break;
+      if (!indirect)
+	{
+	  error ("missing indirect call in speculative call sequence");
+	  return true;
+	}
+      if (!indirect->speculative)
+	{
+	  error ("indirect call in speculative call sequence has no "
+		 "speculative flag");
+	  return true;
+	}
+      return false;
+    }
+
+  /* Maximal number of targets.  We probably will never want to have more than
+     this.  */
+  const unsigned int num = 256;
+  cgraph_edge *direct_calls[num];
+  ipa_ref *refs[num];
+
+  for (unsigned int i = 0; i < num; i++)
+    {
+      direct_calls[i] = NULL;
+      refs[i] = NULL;
+    }
+
+  for (cgraph_edge *direct = node->callees; direct;
+       direct = direct->next_callee)
+    if (direct->call_stmt == stmt && direct->lto_stmt_uid == lto_stmt_uid)
+      {
+	if (!direct->speculative)
+	  {
+	    error ("direct call to %s in speculative call sequence has no "
+		   "speculative flag", direct->callee->dump_name ());
+	    return true;
+	  }
+	if (direct->speculative_id >= num)
+	  {
+	    error ("direct call to %s in speculative call sequence has "
+		   "speculative_uid %i out of range",
+		   direct->callee->dump_name (), direct->speculative_id);
+	    return true;
+	  }
+	if (direct_calls[direct->speculative_id])
+	  {
+	    error ("duplicate direct call to %s in speculative call sequence "
+		   "with speculative_uid %i",
+		   direct->callee->dump_name (), direct->speculative_id);
+	    return true;
+	  }
+	direct_calls[direct->speculative_id] = direct;
+      }
+
+  ipa_ref *ref;
+  for (int i = 0; node->iterate_reference (i, ref); i++)
+    if (ref->speculative
+	&& ref->stmt == stmt && ref->lto_stmt_uid == lto_stmt_uid)
+      {
+	if (ref->speculative_id >= num)
+	  {
+	    error ("direct call to %s in speculative call sequence has "
+		   "speculative_uid %i out of range",
+		   ref->referred->dump_name (), ref->speculative_id);
+	    return true;
+	  }
+	if (refs[ref->speculative_id])
+	  {
+	    error ("duplicate reference %s in speculative call sequence "
+		   "with speculative_uid %i",
+		   ref->referred->dump_name (), ref->speculative_id);
+	    return true;
+	  }
+	refs[ref->speculative_id] = ref;
+      }
+
+  int num_targets = 0;
+  for (unsigned int i = 0 ; i < num ; i++)
+    {
+      if (refs[i] && !direct_calls[i])
+	{
+	  error ("missing direct call for speculation %i", i);
+	  return true;
+	}
+      if (!refs[i] && direct_calls[i])
+	{
+	  error ("missing ref for speculation %i", i);
+	  return true;
+	}
+      if (refs[i] != NULL)
+	num_targets++;
+    }
+
+  if (num_targets != indirect->num_speculative_call_targets_p ())
+    {
+      error ("number of speculative targets %i mismatched with "
+	     "num_speculative_targets %i",
+	     num_targets,
+	     indirect->num_speculative_call_targets_p ());
+      return true;
+    }
+  return false;
+}
+
 /* Verify cgraph nodes of given cgraph node.  */
 DEBUG_FUNCTION void
 cgraph_node::verify_node (void)
@@ -3320,6 +3443,10 @@  cgraph_node::verify_node (void)
 	  error ("edge has both cal_stmt and lto_stmt_uid set");
 	  error_found = true;
 	}
+      if (e->speculative
+	  && verify_speculative_call (e->caller, e->call_stmt, e->lto_stmt_uid,
+				      NULL))
+	error_found = true;
     }
   for (e = indirect_calls; e; e = e->next_callee)
     {
@@ -3342,7 +3469,24 @@  cgraph_node::verify_node (void)
 	  fprintf (stderr, "\n");
 	  error_found = true;
 	}
+      if (e->speculative
+	  && verify_speculative_call (e->caller, e->call_stmt, e->lto_stmt_uid,
+				      e))
+	error_found = true;
     }
+  for (i = 0; iterate_reference (i, ref); i++)
+    {
+      if (ref->stmt && ref->lto_stmt_uid)
+	{
+	  error ("reference has both cal_stmt and lto_stmt_uid set");
+	  error_found = true;
+	}
+      if (ref->speculative
+	  && verify_speculative_call (this, ref->stmt,
+				      ref->lto_stmt_uid, NULL))
+	error_found = true;
+    }
+
   if (!callers && inlined_to)
     {
       error ("inlined_to pointer is set but no predecessors found");
@@ -3519,13 +3663,6 @@  cgraph_node::verify_node (void)
 	/* No CFG available?!  */
 	gcc_unreachable ();
 
-      for (i = 0; iterate_reference (i, ref); i++)
-	if (ref->stmt && ref->lto_stmt_uid)
-	  {
-	    error ("reference has both cal_stmt and lto_stmt_uid set");
-	    error_found = true;
-	  }
-
       for (e = callees; e; e = e->next_callee)
 	{
 	  if (!e->aux && !e->speculative)
diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
index 03272f20987..670d9e2fb73 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -864,104 +864,104 @@  ipa_profile (void)
 		}
 
 	      unsigned speculative_id = 0;
-	      bool speculative_found = false;
 	      for (unsigned i = 0; i < spec_count; i++)
-	      {
-	      speculative_call_target item
-		= csum->speculative_call_targets[i];
-	      n2 = find_func_by_profile_id (item.target_id);
-	      if (n2)
 		{
-		  if (dump_file)
-		    {
-		      fprintf (dump_file, "Indirect call -> direct call from"
-			       " other module %s => %s, prob %3.2f\n",
-			       n->dump_name (),
-			       n2->dump_name (),
-			       item.target_probability
-				 / (float) REG_BR_PROB_BASE);
-		    }
-		  if (item.target_probability < REG_BR_PROB_BASE / 2)
-		    {
-		      nuseless++;
-		      if (dump_file)
-			fprintf (dump_file,
-				 "Not speculating: probability is too low.\n");
-		    }
-		  else if (!e->maybe_hot_p ())
-		    {
-		      nuseless++;
-		      if (dump_file)
-			fprintf (dump_file,
-				 "Not speculating: call is cold.\n");
-		    }
-		  else if (n2->get_availability () <= AVAIL_INTERPOSABLE
-			   && n2->can_be_discarded_p ())
-		    {
-		      nuseless++;
-		      if (dump_file)
-			fprintf (dump_file,
-				 "Not speculating: target is overwritable "
-				 "and can be discarded.\n");
-		    }
-		  else if (!check_argument_count (n2, e))
+		  speculative_call_target item
+		    = csum->speculative_call_targets[i];
+		  n2 = find_func_by_profile_id (item.target_id);
+		  if (n2)
 		    {
-		      nmismatch++;
 		      if (dump_file)
-			fprintf (dump_file,
-				 "Not speculating: "
-				 "parameter count mismatch\n");
+			{
+			  fprintf (dump_file,
+				   "Indirect call -> direct call from"
+				   " other module %s => %s, prob %3.2f\n",
+				   n->dump_name (),
+				   n2->dump_name (),
+				   item.target_probability
+				     / (float) REG_BR_PROB_BASE);
+			}
+		      if (item.target_probability < REG_BR_PROB_BASE / 2)
+			{
+			  nuseless++;
+			  if (dump_file)
+			    fprintf (dump_file,
+				     "Not speculating: "
+				     "probability is too low.\n");
+			}
+		      else if (!e->maybe_hot_p ())
+			{
+			  nuseless++;
+			  if (dump_file)
+			    fprintf (dump_file,
+				     "Not speculating: call is cold.\n");
+			}
+		      else if (n2->get_availability () <= AVAIL_INTERPOSABLE
+			       && n2->can_be_discarded_p ())
+			{
+			  nuseless++;
+			  if (dump_file)
+			    fprintf (dump_file,
+				     "Not speculating: target is overwritable "
+				     "and can be discarded.\n");
+			}
+		      else if (!check_argument_count (n2, e))
+			{
+			  nmismatch++;
+			  if (dump_file)
+			    fprintf (dump_file,
+				     "Not speculating: "
+				     "parameter count mismatch\n");
+			}
+		      else if (e->indirect_info->polymorphic
+			       && !opt_for_fn (n->decl, flag_devirtualize)
+			       && !possible_polymorphic_call_target_p (e, n2))
+			{
+			  nimpossible++;
+			  if (dump_file)
+			    fprintf (dump_file,
+				     "Not speculating: "
+				     "function is not in the polymorphic "
+				     "call target list\n");
+			}
+		      else
+			{
+			  /* Target may be overwritable, but profile says that
+			     control flow goes to this particular implementation
+			     of N2.  Speculate on the local alias to allow
+			     inlining.  */
+			  if (!n2->can_be_discarded_p ())
+			    {
+			      cgraph_node *alias;
+			      alias = dyn_cast<cgraph_node *>
+				   (n2->noninterposable_alias ());
+			      if (alias)
+				n2 = alias;
+			    }
+			  nconverted++;
+			  e->make_speculative (n2,
+					       e->count.apply_probability (
+						 item.target_probability),
+					       speculative_id,
+					       item.target_probability);
+			  update = true;
+			  speculative_id++;
+			}
 		    }
-		  else if (e->indirect_info->polymorphic
-			   && !opt_for_fn (n->decl, flag_devirtualize)
-			   && !possible_polymorphic_call_target_p (e, n2))
+		  else
 		    {
-		      nimpossible++;
 		      if (dump_file)
 			fprintf (dump_file,
-				 "Not speculating: "
-				 "function is not in the polymorphic "
-				 "call target list\n");
-		    }
-		  else
-		    {
-		      /* Target may be overwritable, but profile says that
-			 control flow goes to this particular implementation
-			 of N2.  Speculate on the local alias to allow inlining.
-		       */
-		      if (!n2->can_be_discarded_p ())
-			{
-			  cgraph_node *alias;
-			  alias = dyn_cast<cgraph_node *> (n2->noninterposable_alias ());
-			  if (alias)
-			    n2 = alias;
-			}
-		      nconverted++;
-		      e->make_speculative (n2,
-					   e->count.apply_probability (
-					     item.target_probability),
-					   speculative_id,
-					   item.target_probability);
-		      update = true;
-		      speculative_id++;
-		      speculative_found = true;
+				 "Function with profile-id %i not found.\n",
+				 item.target_id);
+		      nunknown++;
 		    }
 		}
-	      else
-		{
-		  if (dump_file)
-		    fprintf (dump_file, "Function with profile-id %i not found.\n",
-			     item.target_id);
-		  nunknown++;
-		}
-	       }
-	     if (speculative_found)
-	       e->indirect_info->num_speculative_call_targets = speculative_id;
 	    }
-	 }
-       if (update)
-	 ipa_update_overall_fn_summary (n);
-     }
+	}
+      if (update)
+	ipa_update_overall_fn_summary (n);
+    }
   if (node_map_initialized)
     del_node_map ();
   if (dump_file && nindirect)