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

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

On Mon, Oct 6, 2014 at 1:31 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> Yes. That's doable. I'm just not sure if top 2 entries are enough. I
>> can see from your example that it will be useful for BBs having
>> multiple case stmt.
>
> I suppose we want real histogram profiler here.  I.e. something that is given
> value ranges and fill in the counts depending on value ranges.
> Other case where your profiler may be useful is the division/modulo conversion.
> For hashtables divisions/modules are often small sets of prime numbers.

yes, tracking value ranges sounds interesting. The topn is also
configurable to track more values, but it will become more expensive
to use.

David

>>
>> Yes. I'm looking at this too. I actually ported the fdo-use part of
>> the patch and it passes spec2006. But it performs the transformation
>> using the old way (in gimple_ic_transform()) -- so I did not send out
>> that patch.
>> I'm trying to do this in the new way (in ipa-profile()). In any case,
>> that will be a separated patch.
>>
>> Also, the switch expansion profile should be also a separated patch.
>>
>> Is it ok to commit these two patches now?
>
> Yes, it is OK, thanks!
>
> Honza
>>
>> Thanks,
>>
>> -Rong
>>
>> >> >
>> >> > 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;
>> >> >