Implement speculative call verifier
authorJan Hubicka <jh@suse.cz>
Sun, 19 Jan 2020 15:41:11 +0000 (16:41 +0100)
committerJan Hubicka <jh@suse.cz>
Sun, 19 Jan 2020 15:41:11 +0000 (16:41 +0100)
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.

* 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.

gcc/ChangeLog
gcc/cgraph.c
gcc/ipa-profile.c

index 0e24b4a64721432a0cbd19ce447030096e12e771..a50847992f9931e57f3e63be5440252da53f710d 100644 (file)
@@ -1,3 +1,13 @@
+2020-01-18  Jan Hubicka  <hubicka@ucw.cz>
+
+       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.
+
 2020-01-18  Jan Hubicka  <hubicka@ucw.cz>
 
        PR lto/93318
index c442f334fe24632022ff854a385c04c22424d5b1..187f6ed30ba357b7bfaddf6039e72e205f3ac366 100644 (file)
@@ -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)
index 03272f2098766b25f03051eca4cfd643fb7c43c3..670d9e2fb73a32c6ea98741d8c002e729011f101 100644 (file)
@@ -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)