This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Time profiler - phase 1


> +2013-10-29  Martin Liska  <marxin.liska@gmail.com>
> +						Jan Hubicka  <jh@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  <steven@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)
> @@ -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.
> +2013-10-28  Martin Liska	<marxin.liska@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.

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.

Honza


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]