https://gcc.gnu.org/g:483acdc188012cf5b1fc14a82402764c920470aa

commit r16-1772-g483acdc188012cf5b1fc14a82402764c920470aa
Author: Jan Hubicka <hubi...@ucw.cz>
Date:   Sun Jun 29 07:05:16 2025 +0200

    Impove diagnostics of mismatched discriminators in auto-profile
    
    We are missing discriminator info in auto-profiles, for example in 
exchange2.
    I am not sure why, since I see the info still present in dwarf2out, so it 
may
    be bug at create_gcov side.
    
    This patch makes the workaround to ouptput better diagnostics (to actually 
show
    the soruce location).  This needs promotion of location info through the 
inline
    stack API, so I turned it from pair to actual structure.  Overall I think 
pairs
    are overused in this source and makes it harder to read.
    
    Bootstrapped/regtested x86_64-linux, comitted.
    
    gcc/ChangeLog:
    
            * auto-profile.cc (struct decl_lineno): Turn to structure; add
            location.
            (dump_inline_stack): Update.
            (get_inline_stack): Update.
            (get_relative_location_for_locus): Fixup formating.
            (function_instance::get_function_instance_by_decl): Add
            LOCATION parameter; improve dumping.
            (autofdo_source_profile::get_callsite_total_count): Improve dumping;
            update.
            (walk_block): Update.
            (autofdo_source_profile::offline_unrealized_inlines): Update.
            (autofdo_source_profile::get_count_info): Update.

Diff:
---
 gcc/auto-profile.cc | 126 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 78 insertions(+), 48 deletions(-)

diff --git a/gcc/auto-profile.cc b/gcc/auto-profile.cc
index 7cf1e8f1b815..44e7faa8fee6 100644
--- a/gcc/auto-profile.cc
+++ b/gcc/auto-profile.cc
@@ -144,10 +144,17 @@ private:
 };
 
 /* Represent a source location: (function_decl, lineno).  */
-typedef std::pair<tree, unsigned> decl_lineno;
+struct decl_lineno
+{
+  tree decl;
+  /* Relative locations stored in auto-profile.  */
+  unsigned int afdo_loc;
+  /* Actual location afdo_loc was computed from used to output diagnostics.  */
+  location_t location;
+};
 
 /* Represent an inline stack. vector[0] is the leaf node.  */
-typedef auto_vec<decl_lineno> inline_stack;
+typedef auto_vec<decl_lineno, 20> inline_stack;
 
 /* String array that stores function names.  */
 typedef auto_vec<char *> string_vector;
@@ -273,9 +280,11 @@ public:
   }
 
   /* Traverse callsites of the current function_instance to find one at the
-     location of LINENO and callee name represented in DECL.  */
+     location of LINENO and callee name represented in DECL.
+     LOCATION should match LINENO and is used to output diagnostics.  */
   function_instance *get_function_instance_by_decl (unsigned lineno,
-                                                    tree decl) const;
+                                                   tree decl,
+                                                   location_t location) const;
 
   /* Merge profile of clones.  Note that cloning hasnt been performed when
      we annotate the CFG (at this stage).  */
@@ -620,8 +629,8 @@ dump_inline_stack (FILE *f, inline_stack *stack)
     {
       fprintf (f, "%s%s:",
               first ? "" : "; ",
-              IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (p.first)));
-      dump_afdo_loc (f, p.second);
+              IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (p.decl)));
+      dump_afdo_loc (f, p.afdo_loc);
       first = false;
     }
   fprintf (f, "\n");
@@ -649,11 +658,11 @@ get_inline_stack (location_t locus, inline_stack *stack,
 
           tree decl = get_function_decl_from_block (block);
           stack->safe_push (
-              std::make_pair (decl, get_combined_location (locus, decl)));
+             {decl, get_combined_location (locus, decl), locus});
           locus = tmp_locus;
         }
     }
-  stack->safe_push (std::make_pair (fn, get_combined_location (locus, fn)));
+  stack->safe_push ({fn, get_combined_location (locus, fn), locus});
 }
 
 /* Same as get_inline_stack for a given node which may be
@@ -696,7 +705,7 @@ get_relative_location_for_locus (tree fn, tree block, 
location_t locus)
        block = BLOCK_SUPERCONTEXT (block))
     if (inlined_function_outer_scope_p (block))
       return get_combined_location (locus,
-                                    get_function_decl_from_block (block));
+                                   get_function_decl_from_block (block));
   return get_combined_location (locus, fn);
 }
 
@@ -806,7 +815,8 @@ function_instance::~function_instance ()
 
 function_instance *
 function_instance::get_function_instance_by_decl (unsigned lineno,
-                                                  tree decl) const
+                                                 tree decl,
+                                                 location_t location) const
 {
   int func_name_idx = afdo_string_table->get_index_by_decl (decl);
   if (func_name_idx != -1)
@@ -816,32 +826,27 @@ function_instance::get_function_instance_by_decl 
(unsigned lineno,
       if (ret != callsites.end ())
         return ret->second;
     }
-  if (dump_file)
-    {
-      for (auto const &iter : callsites)
-       if (iter.first.first == lineno)
-         {
-           fprintf (dump_file, "Looking for %s:",
-                    IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
-           dump_afdo_loc (dump_file, lineno);
-           fprintf (dump_file, " in ");
-           this->dump_inline_stack (dump_file);
-           fprintf (dump_file, " has mismatching call at smae loc to %s\n",
-                    afdo_string_table->get_name (iter.first.second));
-         }
-    }
-  /* ??? If this is used to determine count, we will end up over-eastimating it
-     if offlined function has multiple callers.  */
   if (DECL_FROM_INLINE (decl))
     {
       function_instance
                *ret =  get_function_instance_by_decl (lineno,
-                                               DECL_ABSTRACT_ORIGIN (decl));
-      if (ret && dump_file)
-       fprintf (dump_file, "Passing to offline instance:%s\n",
-                IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
+                                               DECL_ABSTRACT_ORIGIN (decl),
+                                               location);
       return ret;
     }
+  if (dump_enabled_p ())
+    {
+      for (auto const &iter : callsites)
+       if (iter.first.first == lineno)
+         dump_printf_loc (MSG_NOTE | MSG_PRIORITY_INTERNALS,
+                          dump_user_location_t::from_location_t (location),
+                          "auto-profile has mismatched function name %s"
+                          " instaed of %s at loc %i:%i",
+                          afdo_string_table->get_name (iter.first.second),
+                          IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)),
+                          lineno << 16,
+                          lineno & 65535);
+    }
 
   return NULL;
 }
@@ -1487,7 +1492,8 @@ walk_block (tree fn, function_instance *s, tree block)
                       BLOCK_SOURCE_LOCATION (block));
       function_instance *ns
        = s->get_function_instance_by_decl
-                 (loc, BLOCK_ABSTRACT_ORIGIN (block));
+                 (loc, BLOCK_ABSTRACT_ORIGIN (block),
+                  BLOCK_SOURCE_LOCATION (block));
       if (!ns)
        {
          if (dump_file)
@@ -1545,7 +1551,8 @@ autofdo_source_profile::offline_unrealized_inlines ()
                  fprintf (dump_file, "Marking realized %s\n",
                           afdo_string_table->get_name (index));
                f->set_realized ();
-               if (DECL_INITIAL (n->decl) != error_mark_node)
+               if (DECL_INITIAL (n->decl)
+                   && DECL_INITIAL (n->decl) != error_mark_node)
                  walk_block (n->decl, f, DECL_INITIAL (n->decl));
              }
            if (n->next_sharing_asm_name)
@@ -1729,7 +1736,7 @@ autofdo_source_profile::get_count_info (location_t 
gimple_loc,
   function_instance *s = get_function_instance_by_inline_stack (stack);
   if (s == NULL)
     return false;
-  return s->get_count_info (stack[0].second, info);
+  return s->get_count_info (stack[0].afdo_loc, info);
 }
 
 /* Update value profile INFO for STMT from the inlined indirect callsite.
@@ -1836,7 +1843,7 @@ autofdo_source_profile::get_callsite_total_count (
     struct cgraph_edge *edge) const
 {
   inline_stack stack;
-  stack.safe_push (std::make_pair (edge->callee->decl, 0));
+  stack.safe_push ({edge->callee->decl, 0, UNKNOWN_LOCATION});
 
   get_inline_stack_in_node (gimple_location (edge->call_stmt), &stack,
                            edge->caller);
@@ -1948,35 +1955,58 @@ 
autofdo_source_profile::get_function_instance_by_inline_stack (
     const inline_stack &stack) const
 {
   name_function_instance_map::const_iterator iter = map_.find (
-      afdo_string_table->get_index_by_decl (stack[stack.length () - 1].first));
+      afdo_string_table->get_index_by_decl (stack[stack.length () - 1].decl));
   if (iter == map_.end ())
     {
       if (dump_file)
        fprintf (dump_file, "No offline instance for %s\n",
                 IDENTIFIER_POINTER
-                  (DECL_ASSEMBLER_NAME (stack[stack.length () - 1].first)));
+                  (DECL_ASSEMBLER_NAME (stack[stack.length () - 1].decl)));
       return NULL;
     }
   function_instance *s = iter->second;
   for (unsigned i = stack.length () - 1; i > 0; i--)
     {
       function_instance *os = s;
-      s = s->get_function_instance_by_decl (stack[i].second,
-                                           stack[i - 1].first);
-      /* Try lost locus.  */
+      s = s->get_function_instance_by_decl (stack[i].afdo_loc,
+                                           stack[i - 1].decl,
+                                           stack[i].location);
+      /* Try lost discriminator.  */
       if (!s)
-       s = os->get_function_instance_by_decl (stack[i].second & ~65535,
-                                               stack[i - 1].first);
-      if (s == NULL)
        {
-         if (dump_file)
+         s = os->get_function_instance_by_decl (stack[i].afdo_loc & ~65535,
+                                                stack[i - 1].decl,
+                                                stack[i].location);
+         if (s && dump_enabled_p ())
            {
-             fprintf (dump_file, "No instance for %s at loc ",
-                      IDENTIFIER_POINTER
-                       (DECL_ASSEMBLER_NAME (stack[i - 1].first)));
-             dump_afdo_loc (dump_file, stack[i].second);
-             fprintf (dump_file, "\n");
+             dump_printf_loc (MSG_NOTE | MSG_PRIORITY_INTERNALS,
+                              dump_user_location_t::from_location_t
+                                (stack[i].location),
+                               "auto-profile apparently has a missing "
+                               "discriminator for inlined call "
+                               "of %s at relative loc %i:%i\n",
+                              IDENTIFIER_POINTER
+                               (DECL_ASSEMBLER_NAME (stack[i - 1].decl)),
+                              stack[i].afdo_loc >> 16,
+                              stack[i].afdo_loc & 65535);
            }
+       }
+      if (s == NULL)
+       {
+         /* afdo inliner extends the stack by last entry with unknown
+            location while chekcing if function was inlined during train run.
+            We do not want to print diagnostics about every function
+            which is not inlined.  */
+         if (s && dump_enabled_p () && stack[i].location != UNKNOWN_LOCATION)
+           dump_printf_loc (MSG_NOTE | MSG_PRIORITY_INTERNALS,
+                            dump_user_location_t::from_location_t
+                              (stack[i].location),
+                             "auto-profile has no inlined function instance "
+                             "for inlined call of %s at relative loc %i:%i\n",
+                            IDENTIFIER_POINTER
+                             (DECL_ASSEMBLER_NAME (stack[i - 1].decl)),
+                            stack[i].afdo_loc >> 16,
+                            stack[i].afdo_loc & 65535);
          return NULL;
        }
     }

Reply via email to