Patchwork [GOOGLE] Update AutoFDO annotation

login
register
mail settings
Submitter Dehao Chen
Date Aug. 22, 2013, 10:56 p.m.
Message ID <CAO2gOZXTn53TOf4HJu_SHpQYg=Kinih_nixpe8k_AkCcrRpq3g@mail.gmail.com>
Download mbox | patch
Permalink /patch/269226/
State New
Headers show

Comments

Dehao Chen - Aug. 22, 2013, 10:56 p.m.
This patch has 2 changes:

1. Now that we have discriminator for inlined callsite, we don't need
special handling for callsite location any more.
2. If a source line is mapped to multiple BBs, only the first BB will
be annotated.
3. Before actual annotation, mark everythin BB/edge as not annotated.

Bootstrapped and passed regression test.

OK for google branch?

Thanks,
Dehao
Xinliang David Li - Aug. 26, 2013, 11:11 p.m.
Can you add missing documentation on functions like ...:get_count_info
-- documenting return value etc.  Also it might be better to avoid
using 'set' as the local variable name. Change it to something more
specific.

thanks,

David

On Thu, Aug 22, 2013 at 3:56 PM, Dehao Chen <dehao@google.com> wrote:
> This patch has 2 changes:
>
> 1. Now that we have discriminator for inlined callsite, we don't need
> special handling for callsite location any more.
> 2. If a source line is mapped to multiple BBs, only the first BB will
> be annotated.
> 3. Before actual annotation, mark everythin BB/edge as not annotated.
>
> Bootstrapped and passed regression test.
>
> OK for google branch?
>
> Thanks,
> Dehao
>
> Index: gcc/auto-profile.c
> ===================================================================
> --- gcc/auto-profile.c (revision 201927)
> +++ gcc/auto-profile.c (working copy)
> @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include <string.h>
>  #include <map>
>  #include <vector>
> +#include <set>
>
>  #include "config.h"
>  #include "system.h"
> @@ -100,6 +101,10 @@ typedef std::map<unsigned, gcov_type> icall_target
>     (execution_count, value_profile_histogram).  */
>  typedef std::pair<gcov_type, icall_target_map> count_info;
>
> +/* Set of inline_stack. Used to track if the profile is already used to
> +   annotate the program.  */
> +typedef std::set<inline_stack> location_set;
> +
>  struct string_compare
>  {
>    bool operator() (const char *a, const char *b) const
> @@ -202,7 +207,8 @@ class autofdo_source_profile {
>    const function_instance *get_function_instance_by_decl (tree decl) const;
>    /* Find profile info for a given gimple STMT. If found, store the profile
>       info in INFO, and return true; otherwise return false.  */
> -  bool get_count_info (gimple stmt, count_info *info) const;
> +  bool get_count_info (gimple stmt, count_info *info,
> +       const location_set *set) const;
>    /* Find total count of the callee of EDGE.  */
>    gcov_type get_callsite_total_count (struct cgraph_edge *edge) const;
>
> @@ -284,17 +290,13 @@ static const char *get_original_name (const char *
>
>  /* Return the combined location, which is a 32bit integer in which
>     higher 16 bits stores the line offset of LOC to the start lineno
> -   of DECL, The lower 16 bits stores the discrimnator of LOC if
> -   USE_DISCR is true, otherwise 0.  */
> +   of DECL, The lower 16 bits stores the discrimnator.  */
>
>  static unsigned
> -get_combined_location (location_t loc, tree decl, bool use_discr)
> +get_combined_location (location_t loc, tree decl)
>  {
> -  if (use_discr)
> -    return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16)
> -   | get_discriminator_from_locus (loc);
> -  else
> -    return (LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16;
> +  return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16)
> + | get_discriminator_from_locus (loc);
>  }
>
>  /* Return the function decl of a given lexical BLOCK.  */
> @@ -316,7 +318,7 @@ get_function_decl_from_block (tree block)
>  }
>
>  static void
> -get_inline_stack (gimple stmt, bool use_discr, inline_stack *stack)
> +get_inline_stack (gimple stmt, inline_stack *stack)
>  {
>    location_t locus = gimple_location (stmt);
>    if (LOCATION_LOCUS (locus) == UNKNOWN_LOCATION)
> @@ -337,14 +339,13 @@ static void
>
>        tree decl = get_function_decl_from_block (block);
>        stack->push_back (std::make_pair (
> -  decl, get_combined_location (locus, decl, level == 0 && use_discr)));
> +  decl, get_combined_location (locus, decl)));
>        locus = tmp_locus;
>        level++;
>      }
>    stack->push_back (std::make_pair (
>        current_function_decl,
> -      get_combined_location (locus, current_function_decl,
> -     level == 0 && use_discr)));
> +      get_combined_location (locus, current_function_decl)));
>  }
>
>
> @@ -523,14 +524,16 @@ const function_instance *autofdo_source_profile::g
>    return ret == map_.end() ? NULL : ret->second;
>  }
>
> -bool autofdo_source_profile::get_count_info (gimple stmt,
> -     count_info *info) const
> +bool autofdo_source_profile::get_count_info (gimple stmt, count_info *info,
> +     const location_set *set) const
>  {
>    if (LOCATION_LOCUS (gimple_location (stmt)) == cfun->function_end_locus)
>      return false;
>
>    inline_stack stack;
> -  get_inline_stack (stmt, true, &stack);
> +  get_inline_stack (stmt, &stack);
> +  if (set && set->find(stack) != set->end())
> +    return false;
>    if (stack.size () == 0)
>      return false;
>    const function_instance *s = get_function_instance_by_inline_stack (stack);
> @@ -544,7 +547,7 @@ gcov_type autofdo_source_profile::get_callsite_tot
>  {
>    inline_stack stack;
>    stack.push_back (std::make_pair(edge->callee->symbol.decl, 0));
> -  get_inline_stack (edge->call_stmt, false, &stack);
> +  get_inline_stack (edge->call_stmt, &stack);
>
>    const function_instance *s = get_function_instance_by_inline_stack (stack);
>    if (s == NULL)
> @@ -821,7 +824,7 @@ afdo_vpt (gimple stmt, const icall_target_map &map
>     on statements.  */
>
>  static gcov_type
> -afdo_get_bb_count (basic_block bb)
> +afdo_get_bb_count (basic_block bb, location_set *set)
>  {
>    gimple_stmt_iterator gsi;
>    gcov_type max_count = 0;
> @@ -831,7 +834,7 @@ static gcov_type
>      {
>        count_info info;
>        gimple stmt = gsi_stmt (gsi);
> -      if (afdo_source_profile->get_count_info (stmt, &info))
> +      if (afdo_source_profile->get_count_info (stmt, &info, set))
>   {
>    if (info.first > max_count)
>      max_count = info.first;
> @@ -840,6 +843,14 @@ static gcov_type
>      afdo_vpt (stmt, info.second);
>   }
>      }
> +
> +  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +    {
> +      inline_stack stack;
> +      get_inline_stack (gsi_stmt (gsi), &stack);
> +      if (stack.size() > 0)
> + set->insert(stack);
> +    }
>    if (has_annotated)
>      {
>        bb->flags |= BB_ANNOTATED;
> @@ -1220,9 +1231,22 @@ afdo_annotate_cfg (void)
>    ENTRY_BLOCK_PTR->count = s->head_count ();
>    gcov_type max_count = ENTRY_BLOCK_PTR->count;
>
> +  location_set set;
> +
>    FOR_EACH_BB (bb)
>      {
> -      bb->count = afdo_get_bb_count (bb);
> +      edge e;
> +      edge_iterator ei;
> +
> +      bb->count = 0;
> +      bb->flags &= (~BB_ANNOTATED);
> +      FOR_EACH_EDGE (e, ei, bb->succs)
> + {
> +  e->count = 0;
> +  e->flags &= (~EDGE_ANNOTATED);
> + }
> +
> +      bb->count = afdo_get_bb_count (bb, &set);
>        if (bb->count > max_count)
>   max_count = bb->count;
>      }

Patch

Index: gcc/auto-profile.c
===================================================================
--- gcc/auto-profile.c (revision 201927)
+++ gcc/auto-profile.c (working copy)
@@ -24,6 +24,7 @@  along with GCC; see the file COPYING3.  If not see
 #include <string.h>
 #include <map>
 #include <vector>
+#include <set>

 #include "config.h"
 #include "system.h"
@@ -100,6 +101,10 @@  typedef std::map<unsigned, gcov_type> icall_target
    (execution_count, value_profile_histogram).  */
 typedef std::pair<gcov_type, icall_target_map> count_info;

+/* Set of inline_stack. Used to track if the profile is already used to
+   annotate the program.  */
+typedef std::set<inline_stack> location_set;
+
 struct string_compare
 {
   bool operator() (const char *a, const char *b) const
@@ -202,7 +207,8 @@  class autofdo_source_profile {
   const function_instance *get_function_instance_by_decl (tree decl) const;
   /* Find profile info for a given gimple STMT. If found, store the profile
      info in INFO, and return true; otherwise return false.  */
-  bool get_count_info (gimple stmt, count_info *info) const;
+  bool get_count_info (gimple stmt, count_info *info,
+       const location_set *set) const;
   /* Find total count of the callee of EDGE.  */
   gcov_type get_callsite_total_count (struct cgraph_edge *edge) const;

@@ -284,17 +290,13 @@  static const char *get_original_name (const char *

 /* Return the combined location, which is a 32bit integer in which
    higher 16 bits stores the line offset of LOC to the start lineno
-   of DECL, The lower 16 bits stores the discrimnator of LOC if
-   USE_DISCR is true, otherwise 0.  */
+   of DECL, The lower 16 bits stores the discrimnator.  */

 static unsigned
-get_combined_location (location_t loc, tree decl, bool use_discr)
+get_combined_location (location_t loc, tree decl)
 {
-  if (use_discr)
-    return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16)
-   | get_discriminator_from_locus (loc);
-  else
-    return (LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16;
+  return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16)
+ | get_discriminator_from_locus (loc);
 }

 /* Return the function decl of a given lexical BLOCK.  */
@@ -316,7 +318,7 @@  get_function_decl_from_block (tree block)
 }

 static void
-get_inline_stack (gimple stmt, bool use_discr, inline_stack *stack)
+get_inline_stack (gimple stmt, inline_stack *stack)
 {
   location_t locus = gimple_location (stmt);
   if (LOCATION_LOCUS (locus) == UNKNOWN_LOCATION)
@@ -337,14 +339,13 @@  static void

       tree decl = get_function_decl_from_block (block);
       stack->push_back (std::make_pair (
-  decl, get_combined_location (locus, decl, level == 0 && use_discr)));
+  decl, get_combined_location (locus, decl)));
       locus = tmp_locus;
       level++;
     }
   stack->push_back (std::make_pair (
       current_function_decl,
-      get_combined_location (locus, current_function_decl,
-     level == 0 && use_discr)));
+      get_combined_location (locus, current_function_decl)));
 }


@@ -523,14 +524,16 @@  const function_instance *autofdo_source_profile::g
   return ret == map_.end() ? NULL : ret->second;
 }

-bool autofdo_source_profile::get_count_info (gimple stmt,
-     count_info *info) const
+bool autofdo_source_profile::get_count_info (gimple stmt, count_info *info,
+     const location_set *set) const
 {
   if (LOCATION_LOCUS (gimple_location (stmt)) == cfun->function_end_locus)
     return false;

   inline_stack stack;
-  get_inline_stack (stmt, true, &stack);
+  get_inline_stack (stmt, &stack);
+  if (set && set->find(stack) != set->end())
+    return false;
   if (stack.size () == 0)
     return false;
   const function_instance *s = get_function_instance_by_inline_stack (stack);
@@ -544,7 +547,7 @@  gcov_type autofdo_source_profile::get_callsite_tot
 {
   inline_stack stack;
   stack.push_back (std::make_pair(edge->callee->symbol.decl, 0));
-  get_inline_stack (edge->call_stmt, false, &stack);
+  get_inline_stack (edge->call_stmt, &stack);

   const function_instance *s = get_function_instance_by_inline_stack (stack);
   if (s == NULL)
@@ -821,7 +824,7 @@  afdo_vpt (gimple stmt, const icall_target_map &map
    on statements.  */

 static gcov_type
-afdo_get_bb_count (basic_block bb)
+afdo_get_bb_count (basic_block bb, location_set *set)
 {
   gimple_stmt_iterator gsi;
   gcov_type max_count = 0;
@@ -831,7 +834,7 @@  static gcov_type
     {
       count_info info;
       gimple stmt = gsi_stmt (gsi);
-      if (afdo_source_profile->get_count_info (stmt, &info))
+      if (afdo_source_profile->get_count_info (stmt, &info, set))
  {
   if (info.first > max_count)
     max_count = info.first;
@@ -840,6 +843,14 @@  static gcov_type
     afdo_vpt (stmt, info.second);
  }
     }
+
+  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      inline_stack stack;
+      get_inline_stack (gsi_stmt (gsi), &stack);
+      if (stack.size() > 0)
+ set->insert(stack);
+    }
   if (has_annotated)
     {
       bb->flags |= BB_ANNOTATED;
@@ -1220,9 +1231,22 @@  afdo_annotate_cfg (void)
   ENTRY_BLOCK_PTR->count = s->head_count ();
   gcov_type max_count = ENTRY_BLOCK_PTR->count;

+  location_set set;
+
   FOR_EACH_BB (bb)
     {
-      bb->count = afdo_get_bb_count (bb);
+      edge e;
+      edge_iterator ei;
+
+      bb->count = 0;
+      bb->flags &= (~BB_ANNOTATED);
+      FOR_EACH_EDGE (e, ei, bb->succs)
+ {
+  e->count = 0;
+  e->flags &= (~EDGE_ANNOTATED);
+ }
+
+      bb->count = afdo_get_bb_count (bb, &set);
       if (bb->count > max_count)
  max_count = bb->count;
     }