Re: [PATCH] Stream profile summary histogram through LTO files (issue6782131)

[Jan Hubicka <hubicka@ucw.cz>]

> This patch ensures that the histograms from the profile summary are streamed
> through the LTO files so that the working set can be computed for use in
> downstream optimizations.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
> 
> Thanks,
> Teresa
> 
> 2012-11-28  Teresa Johnson  <tejohnson@google.com>
> 
> 	* lto-cgraph.c (output_profile_summary): Stream out sum_all
>         and histogram.
> 	(input_profile_summary): Stream in sum_all and histogram.
> 	(merge_profile_summaries): Merge sum_all and histogram.
> 	(input_symtab): Call compute_working_sets after merging
>         summaries.
> 	* gcov-io.c (gcov_histo_index): Make extern for compiler.
> 	* gcov-io.h (gcov_histo_index): Ditto.
> 	* profile.c (compute_working_sets): Remove static keyword.
> 	* profile.h (compute_working_sets): Ditto.

OK.
> 
> Index: lto-cgraph.c
> ===================================================================
> --- lto-cgraph.c	(revision 193909)
> +++ lto-cgraph.c	(working copy)
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-streamer.h"
>  #include "gcov-io.h"
>  #include "tree-pass.h"
> +#include "profile.h"

Please update dependencies in Makefile.in
> +      /* Count number of non-zero histogram entries, and fill in a bit vector
> +         of non-zero indices.  */
> +         counters.  */
> +      for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++)
> +        histo_bitvector[bv_ix] = 0;
> +      for (h_ix = 0; h_ix < GCOV_HISTOGRAM_SIZE; h_ix++)
> +        {
> +          if (profile_info->histogram[h_ix].num_counters > 0)
> +            {
> +              histo_bitvector[h_ix / 32] |= 1 << (h_ix % 32);
> +              h_cnt++;
> +            }

I think this would be more readable if you just produced a bitpack instead of doing it
by hand, like into gcov-io.
> +	lto_gcov_summary.sum_all = MAX (lto_gcov_summary.sum_all,
> +					(file_data->profile_info.sum_all
> +					 * scale
> +					 + REG_BR_PROB_BASE / 2)
> +					/ REG_BR_PROB_BASE);

Use RDIV for the scaling.
> -#if IN_LIBGCOV || !IN_GCOV
> +#if !IN_GCOV
>  /* Determine the index into histogram for VALUE. */
>  
> -static unsigned
> +GCOV_LINKAGE unsigned
I would probably go around the trouble of declaring this static in GCOV,
so it is inlined at we do not bload libgcov more than needed.

Thanks,
Honza