Thanks. I'll have this change included in my libgcov.c re-re-factoring patch.
-Rong
On Tue, Nov 12, 2013 at 10:18 AM, Martin Liška <marxin.li...@gmail.com> wrote:
> Hi Rong,
> you are right, that's the only usage and so that you can declare it
> static.
>
> Thank you,
> Martin
>
> On Nov 12, 2013 7:10 PM, "Rong Xu" <x...@google.com> wrote:
>>
>> 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
