A question about the newly added global variable function_counter.
Does it have to be a global? Can I make it a file static, within macro
L_gcov_time_profiler? I don't find a use other than in
__gcov_time_profiler().
thanks,
-Rong
On Mon, Nov 11, 2013 at 9:52 AM, Martin Liška <marxin.li...@gmail.com> wrote:
> 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
