Re: [PATCH] Indirect-call topn targets profiler (instrumentation)

[Xinliang David Li <davidxl at google dot com>]

On Mon, Oct 6, 2014 at 11:22 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Rong,
> Would be possible to use topn profiler to get resonale histograms for switch
> expansion, too?  In that case it may make sense to have value version of it as well.

The underlying topn_profiler can be used the same way as the current
one_value profiler for different scenarios. However, I wonder if it is
useful for switch value profiling -- the arc profiling can do the
same.

David

>
> Otherwise the patch seems OK.  I would implement it myself by introducing separate
> variables holding the topn profiler declaration, but can not think of downsides of
> doing it your way.
>
> The topn profiler should be better on determining the dominating target, so perhaps
> you could look into hooking it into ipa-profile.c.  Extending speculative edges for
> multiple targets ought to be also quite easy - we need to update speculative_call_info
> to collect edges annotated to a given call stmt into a vector instead of taking the
> two references it does now.
>
> It would be also very nice to update it to handle case where all the edges are direct
> so we can use it tospeculatively inlinine functions that can be interposed at runtime.
>
> Thanks,
> Honza
>
> Index: gcc/tree-profile.c
> ===================================================================
> --- gcc/tree-profile.c  (revision 215886)
> +++ gcc/tree-profile.c  (working copy)
> @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "target.h"
>  #include "tree-cfgcleanup.h"
>  #include "tree-nested.h"
> +#include "params.h"
>
>  static GTY(()) tree gcov_type_node;
>  static GTY(()) tree tree_interval_profiler_fn;
> @@ -101,7 +102,10 @@ init_ic_make_global_vars (void)
>      {
>        ic_void_ptr_var
>         = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> -                     get_identifier ("__gcov_indirect_call_callee"),
> +                     get_identifier (
> +                             (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
> +                              "__gcov_indirect_call_topn_callee" :
> +                              "__gcov_indirect_call_callee")),
>                       ptr_void);
>        TREE_PUBLIC (ic_void_ptr_var) = 1;
>        DECL_EXTERNAL (ic_void_ptr_var) = 1;
> @@ -131,7 +135,10 @@ init_ic_make_global_vars (void)
>      {
>        ic_gcov_type_ptr_var
>         = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> -                     get_identifier ("__gcov_indirect_call_counters"),
> +                     get_identifier (
> +                             (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
> +                              "__gcov_indirect_call_topn_counters" :
> +                              "__gcov_indirect_call_counters")),
>                       gcov_type_ptr);
>        TREE_PUBLIC (ic_gcov_type_ptr_var) = 1;
>        DECL_EXTERNAL (ic_gcov_type_ptr_var) = 1;
> @@ -226,8 +233,10 @@ gimple_init_edge_profiler (void)
>                                               ptr_void,
>                                               NULL_TREE);
>           tree_indirect_call_profiler_fn
> -                 = build_fn_decl ("__gcov_indirect_call_profiler_v2",
> -                                        ic_profiler_fn_type);
> +                 = build_fn_decl ( (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
> +                                    "__gcov_indirect_call_topn_profiler":
> +                                    "__gcov_indirect_call_profiler_v2"),
> +                                  ic_profiler_fn_type);
>          }
>        TREE_NOTHROW (tree_indirect_call_profiler_fn) = 1;
>        DECL_ATTRIBUTES (tree_indirect_call_profiler_fn)
> @@ -398,6 +407,12 @@ gimple_gen_ic_profiler (histogram_value value, uns
>    gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>    tree ref_ptr = tree_coverage_counter_addr (tag, base);
>
> +  if ( (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) &&
> +        tag == GCOV_COUNTER_V_INDIR) ||
> +       (!PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) &&
> +        tag == GCOV_COUNTER_ICALL_TOPNV))
> +    return;
> +
>    ref_ptr = force_gimple_operand_gsi (&gsi, ref_ptr,
>                                       true, NULL_TREE, true, GSI_SAME_STMT);
>
> @@ -442,8 +457,7 @@ gimple_gen_ic_func_profiler (void)
>      stmt1: __gcov_indirect_call_profiler_v2 (profile_id,
>                                              &current_function_decl)
>     */
> -  gsi =
> -                                            gsi_after_labels (split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun))));
> +  gsi = gsi_after_labels (split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun))));
>
>    cur_func = force_gimple_operand_gsi (&gsi,
>                                        build_addr (current_function_decl,
> Index: gcc/value-prof.c
> ===================================================================
> --- gcc/value-prof.c    (revision 215886)
> +++ gcc/value-prof.c    (working copy)
> @@ -60,6 +60,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "builtins.h"
>  #include "tree-nested.h"
>  #include "hash-set.h"
> +#include "params.h"
>
>  /* In this file value profile based optimizations are placed.  Currently the
>     following optimizations are implemented (for more detailed descriptions
> @@ -359,6 +360,22 @@ dump_histogram_value (FILE *dump_file, histogram_v
>        }
>        fprintf (dump_file, ".\n");
>        break;
> +    case HIST_TYPE_INDIR_CALL_TOPN:
> +      fprintf (dump_file, "Indirect call topn ");
> +      if (hist->hvalue.counters)
> +       {
> +           int i;
> +
> +           fprintf (dump_file, "accu:%"PRId64, hist->hvalue.counters[0]);
> +           for (i = 1; i < (GCOV_ICALL_TOPN_VAL << 2); i += 2)
> +             {
> +               fprintf (dump_file, " target:%"PRId64 " value:%"PRId64,
> +                       (int64_t) hist->hvalue.counters[i],
> +                       (int64_t) hist->hvalue.counters[i+1]);
> +             }
> +        }
> +      fprintf (dump_file, ".\n");
> +      break;
>      case HIST_TYPE_MAX:
>        gcc_unreachable ();
>     }
> @@ -432,9 +449,14 @@ stream_in_histogram_value (struct lto_input_block
>           break;
>
>         case HIST_TYPE_IOR:
> -  case HIST_TYPE_TIME_PROFILE:
> +        case HIST_TYPE_TIME_PROFILE:
>           ncounters = 1;
>           break;
> +
> +        case HIST_TYPE_INDIR_CALL_TOPN:
> +          ncounters = (GCOV_ICALL_TOPN_VAL << 2) + 1;
> +          break;
> +
>         case HIST_TYPE_MAX:
>           gcc_unreachable ();
>         }
> @@ -1920,8 +1942,12 @@ gimple_indirect_call_to_profile (gimple stmt, hist
>
>    values->reserve (3);
>
> -  values->quick_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_INDIR_CALL,
> -                                                   stmt, callee));
> +  values->quick_push (gimple_alloc_histogram_value (
> +                        cfun,
> +                        PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
> +                          HIST_TYPE_INDIR_CALL_TOPN :
> +                          HIST_TYPE_INDIR_CALL,
> +                       stmt, callee));
>
>    return;
>  }
> @@ -2011,9 +2037,9 @@ gimple_find_values_to_profile (histogram_values *v
>           hist->n_counters = 3;
>           break;
>
> -  case HIST_TYPE_TIME_PROFILE:
> -    hist->n_counters = 1;
> -    break;
> +        case HIST_TYPE_TIME_PROFILE:
> +          hist->n_counters = 1;
> +          break;
>
>         case HIST_TYPE_AVERAGE:
>           hist->n_counters = 2;
> @@ -2023,6 +2049,10 @@ gimple_find_values_to_profile (histogram_values *v
>           hist->n_counters = 1;
>           break;
>
> +        case HIST_TYPE_INDIR_CALL_TOPN:
> +          hist->n_counters = GCOV_ICALL_TOPN_NCOUNTS;
> +          break;
> +
>         default:
>           gcc_unreachable ();
>         }
> Index: gcc/value-prof.h
> ===================================================================
> --- gcc/value-prof.h    (revision 215886)
> +++ gcc/value-prof.h    (working copy)
> @@ -35,6 +35,8 @@ enum hist_type
>    HIST_TYPE_AVERAGE,   /* Compute average value (sum of all values).  */
>    HIST_TYPE_IOR,       /* Used to compute expected alignment.  */
>    HIST_TYPE_TIME_PROFILE, /* Used for time profile */
> +  HIST_TYPE_INDIR_CALL_TOPN, /* Tries to identify the top N most frequently
> +                                called functions in indirect call.  */
>    HIST_TYPE_MAX
>  };
>
> Index: gcc/profile.c
> ===================================================================
> --- gcc/profile.c       (revision 215886)
> +++ gcc/profile.c       (working copy)
> @@ -183,6 +183,7 @@ instrument_values (histogram_values values)
>           break;
>
>         case HIST_TYPE_INDIR_CALL:
> +       case HIST_TYPE_INDIR_CALL_TOPN:
>           gimple_gen_ic_profiler (hist, t, 0);
>           break;
>