This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GOOGLE] Add AutoFDO based indirect call value profiling to GCC


https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c
File gcc/auto-profile.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode161
gcc/auto-profile.c:161: };
Why not sharing the same hist_type enum in value-prof.h?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode171
gcc/auto-profile.c:171: int count;
int --> gcov_type?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode519
gcc/auto-profile.c:519: static void
New line above.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode550
gcc/auto-profile.c:550: if (actual_count < 2)
Are the targets sorted?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode570
gcc/auto-profile.c:570: static void afdo_vpt (gimple stmt, struct
gcov_hist *v, int hist_size)
New line.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode914
gcc/auto-profile.c:914: for the given POS_STACK.  */
Document new parameters.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode1193
gcc/auto-profile.c:1193: gcov_functions[i].stacks[j].hist_size =
gcov_read_unsigned ();
Why is the data not optional?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode1203
gcc/auto-profile.c:1203: file_names[gcov_read_counter()];
missing white space (many other instances)

https://codereview.appspot.com/8814043/diff/1/gcc/tree-inline.c
File gcc/tree-inline.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/tree-inline.c#newcode1833
gcc/tree-inline.c:1833: gcov_type count = afdo_get_bb_count
(copy_basic_block, false);
comment about the 'false' parameter.

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c
File gcc/value-prof.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c#newcode1223
gcc/value-prof.c:1223: if (!strcmp(fname, name))
You can use assembler_name_hash to find the cgraph_node

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c#newcode1626
gcc/value-prof.c:1626: direct_call1 = find_func_by_global_id (val1);
May be add a parameter to find_func_by_global_id (val, is_auto_fdo)?

https://codereview.appspot.com/8814043/

On Tue, Apr 16, 2013 at 8:41 PM, Dehao Chen <dehao@google.com> wrote:
> This patch implements indirect call promotion for AutoFDO.
>
> Bootstrapped and passed gcc regression tests.
>
> Is it okay for gcc-4_7 branch?
>
> Thanks,
> Dehao
>
> http://codereview.appspot.com/8814043
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index d17b250..c217846 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3039,7 +3039,7 @@ coverage.o : coverage.c $(GCOV_IO_H) $(CONFIG_H)
> $(SYSTEM_H) coretypes.h \
>     $(DIAGNOSTIC_CORE_H) intl.h gt-coverage.h $(TARGET_H) $(AUTO_PROFILE_H)
>  auto-profile.o : auto-profile.c $(CONFIG_H) $(SYSTEM_H) $(FLAGS_H) \
>     $(BASIC_BLOCK_H) $(DIAGNOSTIC_CORE_H) $(GCOV_IO_H) $(INPUT_H) $(PROFILE_H) \
> -   $(LANGHOOKS_H) $(OPTS_H) $(TREE_PASS_H) $(CGRAPH_H) $(GIMPLE_H) \
> +   $(LANGHOOKS_H) $(OPTS_H) $(TREE_PASS_H) $(CGRAPH_H) $(GIMPLE_H)
> value-prof.h \
>     $(AUTO_PROFILE_H)
>  cselib.o : cselib.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
>     $(REGS_H) hard-reg-set.h $(FLAGS_H) insn-config.h $(RECOG_H) \
> diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
> index cf17370..38f4209 100644
> --- a/gcc/auto-profile.c
> +++ b/gcc/auto-profile.c
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "gimple.h"
>  #include "cgraph.h"
>  #include "tree-flow.h"
> +#include "value-prof.h"
>  #include "auto-profile.h"
>
>  /* The following routines implements AutoFDO optimization.
> @@ -109,6 +110,8 @@ struct gcov_stack
>    const char *callee_name;
>    struct gcov_callsite_pos *stack;
>    gcov_unsigned_t size;
> +  struct gcov_hist *hist;
> +  gcov_unsigned_t hist_size;
>    gcov_type num_inst;
>    gcov_type count;
>    gcov_type max_count;
> @@ -150,6 +153,24 @@ struct afdo_module
>    char **strings;
>  };
>
> +enum afdo_histogram_type
> +{
> +  CALL_HIST = 1,
> +  STRINGOP_HIST,
> +  DIVMOD_HIST
> +};
> +
> +struct gcov_hist
> +{
> +  enum afdo_histogram_type type;
> +  union
> +    {
> +      const char *func_name;
> +      unsigned long long value;
> +    } value;
> +  int count;
> +};
> +
>  /* Store the file name strings read from the profile data file. */
>  static const char **file_names;
>
> @@ -493,6 +514,64 @@ read_aux_modules (void)
>      }
>  }
>
> +/* From AutoFDO profiles, find values inside STMT for that we want to measure
> +   histograms for indirect-call optimization.  */
> +static void
> +afdo_indirect_call (gimple stmt, struct gcov_hist *values, int hist_size)
> +{
> +  tree callee;
> +  int i, total = 0;
> +  int actual_count = 0;
> +  histogram_value hist;
> +
> +  if (gimple_code (stmt) != GIMPLE_CALL
> +      || gimple_call_fndecl (stmt) != NULL_TREE)
> +    return;
> +
> +  callee = gimple_call_fn (stmt);
> +
> +  for (i = 0; i < hist_size; i++)
> +    if (values[i].type == CALL_HIST)
> +      break;
> +
> +  if (i == hist_size)
> +    return;
> +
> +  hist = gimple_alloc_histogram_value (cfun, HIST_TYPE_INDIR_CALL_TOPN,
> +       stmt, callee);
> +  hist->n_counters = (GCOV_ICALL_TOPN_VAL << 2) + 1;
> +  hist->hvalue.counters =  XNEWVEC (gcov_type, hist->n_counters);
> +  gimple_add_histogram_value (cfun, stmt, hist);
> +
> +  for (i = 0; i < hist_size; i++)
> +    if (values[i].type == CALL_HIST)
> +      {
> + total += values[i].count;
> + if (actual_count < 2)
> +  {
> +    hist->hvalue.counters[actual_count * 2 + 1] =
> + (unsigned long long) values[i].value.func_name;
> +    hist->hvalue.counters[actual_count * 2 + 2] = values[i].count;
> +    actual_count ++;
> +  }
> +      }
> +
> +  hist->hvalue.counters[0] = total;
> +
> +  if (actual_count == 1)
> +    {
> +      hist->hvalue.counters[3] = 0;
> +      hist->hvalue.counters[4] = 0;
> +    }
> +}
> +
> +/* From AutoFDO profiles, find values inside STMT for that we want to measure
> +   histograms and adds them to list VALUES.  */
> +static void afdo_vpt (gimple stmt, struct gcov_hist *v, int hist_size)
> +{
> +  afdo_indirect_call (stmt, v, hist_size);
> +}
> +
>  /* Return the size of the inline stack of the STMT.  */
>
>  static int
> @@ -838,7 +917,8 @@ static bool
>  get_stack_count (struct gcov_callsite_pos *pos_stack,
>   const char *callee_name,
>   int size,
> - gcov_type *count, gcov_type *max_count, gcov_type *num_inst)
> + gcov_type *count, gcov_type *max_count, gcov_type *num_inst,
> + gcov_unsigned_t *hist_size, struct gcov_hist **hist)
>  {
>    int i;
>
> @@ -858,6 +938,11 @@ get_stack_count (struct gcov_callsite_pos *pos_stack,
>        *num_inst = entry->num_inst;
>        if (max_count)
>   *max_count = entry->max_count;
> +      if (hist_size)
> + {
> +  *hist_size = entry->hist_size;
> +  *hist = entry->hist;
> + }
>        return true;
>      }
>    else
> @@ -876,6 +961,11 @@ get_stack_count (struct gcov_callsite_pos *pos_stack,
>    *num_inst = entry->num_inst;
>    if (max_count)
>      *max_count = entry->max_count;
> +  if (hist_size)
> +    {
> +      *hist_size = entry->hist_size;
> +      *hist = entry->hist;
> +    }
>    return true;
>   }
>      }
> @@ -885,6 +975,11 @@ get_stack_count (struct gcov_callsite_pos *pos_stack,
>    *num_inst = 0;
>    if (max_count)
>      *max_count = 0;
> +  if (hist_size)
> +    {
> +      *hist_size = 0;
> +      *hist = 0;
> +    }
>    return false;
>  }
>
> @@ -892,7 +987,8 @@ get_stack_count (struct gcov_callsite_pos *pos_stack,
>     Return FALSE if profile is not found for STMT.  */
>
>  static bool
> -get_stmt_count (gimple stmt, gcov_type *count, gcov_type *num_inst)
> +get_stmt_count (gimple stmt, gcov_type *count, gcov_type *num_inst,
> + gcov_unsigned_t *hist_size, struct gcov_hist **hist)
>  {
>    struct gcov_callsite_pos *pos_stack;
>    int size;
> @@ -910,7 +1006,8 @@ get_stmt_count (gimple stmt, gcov_type *count,
> gcov_type *num_inst)
>
>    get_inline_stack_by_stmt (stmt, current_function_decl, pos_stack, true);
>
> -  return get_stack_count (pos_stack, NULL, size, count, NULL, num_inst);
> +  return get_stack_count (pos_stack, NULL, size, count, NULL, num_inst,
> +  hist_size, hist);
>  }
>
>  /* For a given EDGE, if IS_TOTAL is true, save EDGE->callee's total count
> @@ -938,13 +1035,13 @@ afdo_get_callsite_count (struct cgraph_edge
> *edge, gcov_type *count,
>   get_discriminator_from_locus(gimple_location(edge->call_stmt));
>
>    return get_stack_count (pos_stack, callee_name,
> -  size, count, max_count, &num_inst);
> +  size, count, max_count, &num_inst, NULL, NULL);
>  }
>
>  /* For a given BB, return its execution count.  */
>
>  gcov_type
> -afdo_get_bb_count (basic_block bb)
> +afdo_get_bb_count (basic_block bb, bool annotate_vpt)
>  {
>    gimple_stmt_iterator gsi;
>    gcov_type max_count = 0;
> @@ -953,12 +1050,16 @@ afdo_get_bb_count (basic_block bb)
>    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>      {
>        gcov_type count, num_inst;
> +      gcov_unsigned_t hist_size;
> +      struct gcov_hist *hist;
>        gimple stmt = gsi_stmt (gsi);
> -      if (get_stmt_count (stmt, &count, &num_inst))
> +      if (get_stmt_count (stmt, &count, &num_inst, &hist_size, &hist))
>   {
>    if (count > max_count)
>      max_count = count;
>    has_annotated = true;
> +  if (annotate_vpt && hist_size > 0)
> +    afdo_vpt (stmt, hist, hist_size);
>   }
>      }
>    if (has_annotated)
> @@ -980,7 +1081,7 @@ afdo_annotate_cfg (void)
>
>    FOR_EACH_BB (bb)
>      {
> -      bb->count = afdo_get_bb_count (bb);
> +      bb->count = afdo_get_bb_count (bb, true);
>        if (bb->count > max_count)
>   max_count = bb->count;
>      }
> @@ -992,6 +1093,8 @@ afdo_annotate_cfg (void)
>        afdo_calculate_branch_prob ();
>        profile_status = PROFILE_READ;
>      }
> +  if (flag_value_profile_transformations)
> +    gimple_value_profile_transformations ();
>  }
>
>  extern gcov_working_set_t *gcov_working_sets;
> @@ -1055,7 +1158,7 @@ read_profile (void)
>      xmalloc (function_num * sizeof (struct gcov_function));
>    for (i = 0; i < function_num; i++)
>      {
> -      gcov_functions[i].name = xstrdup (gcov_read_string ());
> +      gcov_functions[i].name = file_names[gcov_read_unsigned ()];
>        gcov_functions[i].file = file_names[gcov_read_unsigned ()];
>        gcov_functions[i].total_count = gcov_read_counter ();
>        gcov_functions[i].entry_count = gcov_read_counter ();
> @@ -1087,6 +1190,22 @@ read_profile (void)
>      }
>    gcov_functions[i].stacks[j].count = gcov_read_counter ();
>    gcov_functions[i].stacks[j].num_inst = gcov_read_counter ();
> +  gcov_functions[i].stacks[j].hist_size = gcov_read_unsigned ();
> +  gcov_functions[i].stacks[j].hist = (struct gcov_hist *)
> +    xmalloc (gcov_functions[i].stacks[j].hist_size
> +     * sizeof (struct gcov_hist));
> +  for (k = 0; k < gcov_functions[i].stacks[j].hist_size; k++)
> +    {
> +      gcov_functions[i].stacks[j].hist[k].type =
> +  (enum afdo_histogram_type) gcov_read_unsigned();
> +      if (gcov_functions[i].stacks[j].hist[k].type == CALL_HIST)
> + gcov_functions[i].stacks[j].hist[k].value.func_name =
> +    file_names[gcov_read_counter()];
> +      else
> + gcov_functions[i].stacks[j].hist[k].value.value =
> +    gcov_read_counter();
> +      gcov_functions[i].stacks[j].hist[k].count = gcov_read_counter();
> +    }
>   }
>      }
>
> @@ -1698,6 +1817,7 @@ auto_profile (void)
>
>        afdo_annotate_cfg ();
>        compute_function_frequency ();
> +      update_ssa (TODO_update_ssa);
>
>        current_function_decl = NULL;
>        pop_cfun ();
> diff --git a/gcc/auto-profile.h b/gcc/auto-profile.h
> index 8e0a670..f7175ff 100644
> --- a/gcc/auto-profile.h
> +++ b/gcc/auto-profile.h
> @@ -43,5 +43,5 @@ extern bool afdo_get_callsite_count (struct
> cgraph_edge *, gcov_type *,
>       gcov_type *, bool);
>
>  /* Calculate basic block count.  */
> -extern gcov_type afdo_get_bb_count (basic_block);
> +extern gcov_type afdo_get_bb_count (basic_block, bool);
>  #endif /* AUTO_PROFILE_H */
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 4657d6a..dab2564 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1665,6 +1665,8 @@ common_handle_option (struct gcc_options *opts,
>   opts->x_flag_unroll_loops = value;
>        if (!opts_set->x_flag_peel_loops)
>   opts->x_flag_peel_loops = value;
> +      if (!opts_set->x_flag_value_profile_transformations)
> + opts->x_flag_value_profile_transformations = value;
>        if (!opts_set->x_flag_inline_functions)
>   opts->x_flag_inline_functions = value;
>        if (!opts_set->x_flag_ipa_cp)
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index f09f201..62180fc 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -1830,7 +1830,7 @@ copy_bb (copy_body_data *id, basic_block bb, int
> frequency_scale,
>      {
>        /* If the same inline happens in the profile-collection binary, use
>   that instance's profile count. Otherwise use the scaled count.  */
> -      gcov_type count = afdo_get_bb_count (copy_basic_block);
> +      gcov_type count = afdo_get_bb_count (copy_basic_block, false);
>        if (copy_basic_block->flags & BB_ANNOTATED)
>   copy_basic_block->count = count;
>        else if (bb->flags & BB_ANNOTATED)
> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
> index 81465bc..c20d13a 100644
> --- a/gcc/value-prof.c
> +++ b/gcc/value-prof.c
> @@ -94,7 +94,7 @@ static bool gimple_ic_transform (gimple);
>
>  /* Allocate histogram value.  */
>
> -static histogram_value
> +histogram_value
>  gimple_alloc_histogram_value (struct function *fun ATTRIBUTE_UNUSED,
>        enum hist_type type, gimple stmt, tree value)
>  {
> @@ -1205,6 +1205,33 @@ find_func_by_funcdef_no (int func_id)
>    return VEC_index (cgraph_node_ptr, cgraph_node_map, func_id);
>  }
>
> +/* Return cgraph node for function with name. We need this because
> +   AutoFDO doesn't record the function id for each function
> +   in the profile.
> +   TODO: need to transform this lookup method to hash table.  */
> +
> +static struct cgraph_node *
> +find_func_by_name (char *name)
> +{
> +  struct cgraph_node *n;
> +  struct cgraph_node *ret = NULL;
> +  int match = 0;
> +
> +  for (n = cgraph_nodes; n; n = n->next)
> +    {
> +      const char *fname = IDENTIFIER_POINTER (decl_assembler_name (n->decl));
> +      if (!strcmp(fname, name))
> + {
> +  match ++;
> +  ret = n;
> + }
> +    }
> +  if (match == 1)
> +    return ret;
> +  else
> +    return NULL;
> +}
> +
>  /* Initialize map of gids (gid -> cgraph node) */
>
>  static htab_t gid_map = NULL;
> @@ -1562,7 +1589,8 @@ gimple_ic_transform_mult_targ (gimple stmt,
> histogram_value histogram)
>    count1 = histogram->hvalue.counters [2];
>    val2 = histogram->hvalue.counters [3];
>    count2 = histogram->hvalue.counters [4];
> -  bb_all = gimple_bb (stmt)->count;
> +  bb_all = flag_auto_profile ? histogram->hvalue.counters[0]
> +     : gimple_bb (stmt)->count;
>    all = bb_all;
>
>    gimple_remove_histogram_value (cfun, stmt, histogram);
> @@ -1592,18 +1620,26 @@ gimple_ic_transform_mult_targ (gimple stmt,
> histogram_value histogram)
>    else
>      prob1 = prob2 = 0;
>
> -  direct_call1 = find_func_by_global_id (val1);
> +  if (flag_auto_profile)
> +    direct_call1 = find_func_by_name ((char *) val1);
> +  else
> +    direct_call1 = find_func_by_global_id (val1);
>
>    if (val2 && (100 * count2 >= all * perc_threshold)
>        && count2 > count_threshold)
> -    direct_call2 = find_func_by_global_id (val2);
> +    {
> +      if (flag_auto_profile)
> + direct_call2 = find_func_by_name ((char *) val2);
> +      else
> + direct_call2 = find_func_by_global_id (val2);
> +    }
>
>    locus = (stmt != NULL) ? gimple_location (stmt)
>        : DECL_SOURCE_LOCATION (current_function_decl);
>    if (direct_call1 == NULL
>        || !check_ic_target (stmt, direct_call1))
>      {
> -      if (flag_opt_info >= OPT_INFO_MAX)
> +      if (flag_opt_info >= OPT_INFO_MAX && !flag_auto_profile)
>          {
>            if (!direct_call1)
>              inform (locus, "Can not find indirect call target decl "
> @@ -1645,9 +1681,12 @@ gimple_ic_transform_mult_targ (gimple stmt,
> histogram_value histogram)
>        print_generic_expr (dump_file, gimple_call_fn (stmt), TDF_SLIM);
>        fprintf (dump_file, "=> ");
>        print_generic_expr (dump_file, direct_call1->decl, TDF_SLIM);
> -      fprintf (dump_file, " (module_id:%d, func_id:%d)\n",
> -               EXTRACT_MODULE_ID_FROM_GLOBAL_ID (val1),
> -               EXTRACT_FUNC_ID_FROM_GLOBAL_ID (val1));
> +      if (flag_auto_profile)
> + fprintf (dump_file, " (%s)\n", (char *) val1);
> +      else
> + fprintf (dump_file, " (module_id:%d, func_id:%d)\n",
> +                 EXTRACT_MODULE_ID_FROM_GLOBAL_ID (val1),
> +                 EXTRACT_FUNC_ID_FROM_GLOBAL_ID (val1));
>        fprintf (dump_file, "Transformation on insn:\n");
>        print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
>        fprintf (dump_file, "==>\n");
> @@ -1683,9 +1722,12 @@ gimple_ic_transform_mult_targ (gimple stmt,
> histogram_value histogram)
>            print_generic_expr (dump_file, gimple_call_fn (stmt), TDF_SLIM);
>            fprintf (dump_file, "=> ");
>            print_generic_expr (dump_file, direct_call2->decl, TDF_SLIM);
> -          fprintf (dump_file, " (module_id:%d, func_id:%d)\n",
> -                   EXTRACT_MODULE_ID_FROM_GLOBAL_ID (val2),
> -                   EXTRACT_FUNC_ID_FROM_GLOBAL_ID (val2));
> +  if (flag_auto_profile)
> +    fprintf (dump_file, " (%s)\n", (char *) val2);
> +  else
> +    fprintf (dump_file, " (module_id:%d, func_id:%d)\n",
> +                     EXTRACT_MODULE_ID_FROM_GLOBAL_ID (val2),
> +                     EXTRACT_FUNC_ID_FROM_GLOBAL_ID (val2));
>            fprintf (dump_file, "Transformation on insn\n");
>            print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
>            fprintf (dump_file, "=>\n");
> diff --git a/gcc/value-prof.h b/gcc/value-prof.h
> index 9425e25..2d84fe9 100644
> --- a/gcc/value-prof.h
> +++ b/gcc/value-prof.h
> @@ -77,6 +77,8 @@ typedef VEC(histogram_value,heap) *histogram_values;
>  extern void gimple_find_values_to_profile (histogram_values *);
>  extern bool gimple_value_profile_transformations (void);
>
> +histogram_value gimple_alloc_histogram_value (struct function *, enum
> hist_type,
> +      gimple stmt, tree);
>  histogram_value gimple_histogram_value (struct function *, gimple);
>  histogram_value gimple_histogram_value_of_type (struct function *, gimple,
>   enum hist_type);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]