On 11 November 2013 15:57, Jan Hubicka <hubi...@ucw.cz> wrote:
>> +2013-10-29 Martin Liska <marxin.li...@gmail.com>
>> + Jan Hubicka <j...@suse.cz>
>> +
>> + * cgraph.c (dump_cgraph_node): Profile dump added.
>> + * cgraph.h (struct cgraph_node): New time profile variable added.
>> + * cgraphclones.c (cgraph_clone_node): Time profile is cloned.
>> + * gcov-io.h (gcov_type): New profiler type introduced.
>> + * ipa-profile.c (lto_output_node): Streaming for time profile added.
>> + (input_node): Time profiler is read from LTO stream.
>> + * predict.c (maybe_hot_count_p): Hot prediction changed.
>> + * profile.c (instrument_values): New case for time profiler added.
>> + (compute_value_histograms): Read of time profile.
>> + * tree-pretty-print.c (dump_function_header): Time profiler is dumped.
>> + * tree-profile.c (init_ic_make_global_vars): Time profiler function
>> added.
>> + (gimple_init_edge_profiler): TP function instrumentation.
>> + (gimple_gen_time_profiler): New.
>> + * value-prof.c (gimple_add_histogram_value): Support for time profiler
>> + added.
>> + (dump_histogram_value): TP type added to dumps.
>> + (visit_hist): More sensitive check that takes TP into account.
>> + (gimple_find_values_to_profile): TP instrumentation.
>> + * value-prof.h (hist_type): New histogram type added.
>> + (struct histogram_value_t): Pointer to struct function added.
>> + * libgcc/Makefile.in: New GCOV merge function for TP added.
>> + * libgcov.c: function_counter variable introduced.
>> + (_gcov_merge_time_profile): New.
>> + (_gcov_time_profiler): New.
>> +
>> 2013-11-05 Steven Bosscher <ste...@gcc.gnu.org>
>>
>>
>> diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
>> index 1260069..eff4b51 100644
>> --- a/gcc/ipa-profile.c
>> +++ b/gcc/ipa-profile.c
>> @@ -465,6 +465,7 @@ ipa_propagate_frequency (struct cgraph_node *node)
>> if (d.maybe_unlikely_executed)
>> {
>> node->frequency = NODE_FREQUENCY_UNLIKELY_EXECUTED;
>> + node->tp_first_run = 0;
>> if (dump_file)
>> fprintf (dump_file, "Node %s promoted to unlikely executed.\n",
>> cgraph_node_name (node));
>
> Why do you put tp_first_run to 0? The function may be unlikely but it still
> may have
> average possition in the program execution. I.e. when it is executed once per
> many
> invocations during the train run)
That makes no sense, I've removed this assignment.
>> @@ -895,9 +907,22 @@ compute_value_histograms (histogram_values values,
>> unsigned cfg_checksum,
>> hist->hvalue.counters = XNEWVEC (gcov_type, hist->n_counters);
>> for (j = 0; j < hist->n_counters; j++)
>> if (aact_count)
>> - hist->hvalue.counters[j] = aact_count[j];
>> - else
>> - hist->hvalue.counters[j] = 0;
>> + hist->hvalue.counters[j] = aact_count[j];
>> + else
>> + hist->hvalue.counters[j] = 0;
>> +
>> + /* Time profiler counter is not related to any statement,
>> + * so that we have to read the counter and set the value to
>> + * the corresponding call graph node. */
>
> Reformat comment to GNU style.
Yes.
>> +2013-10-28 Martin Liska <marxin.li...@gmail.com>
>> +
>> + * gcc.dg/time-profiler-1.c: New test.
>> + * gcc.dg/time-profiler-2.c: Ditto.
>> +
> It is custom to put all changelogs to the beggining of your patch (just
> nitpicking ;))
>> @@ -222,6 +225,18 @@ gimple_init_edge_profiler (void)
>> = tree_cons (get_identifier ("leaf"), NULL,
>> DECL_ATTRIBUTES (tree_indirect_call_profiler_fn));
>>
>> + /* void (*) (gcov_type *, gcov_type, void *) */
>> + time_profiler_fn_type
>> + = build_function_type_list (void_type_node,
>> + gcov_type_ptr, NULL_TREE);
>> + tree_time_profiler_fn
>> + = build_fn_decl ("__gcov_time_profiler",
>> + time_profiler_fn_type);
>> + TREE_NOTHROW (tree_time_profiler_fn) = 1;
>> + DECL_ATTRIBUTES (tree_time_profiler_fn)
>> + = tree_cons (get_identifier ("leaf"), NULL,
>> + DECL_ATTRIBUTES (tree_time_profiler_fn));
>
> We should udpate this code to use set_call_expr_flags but by incremental
> patch.
OK, I will include this change to the phase 2.
> The patch is OK with changes above. Do you have write permissin to commit it?
> If not, just send updated version and I will commit it + follow the write
> access
> instructions on gcc.gnu.org homepage.
Yes, I do have commit right. I will bootstrap the patch, test Inkscape
instrumentation and commit it.
Martin
> Honza
