On Thu, Jun 01, 2017 at 01:35:56PM +0200, Jan Hubicka wrote:
Just some very minor nits.
> Index: final.c
> ===================================================================
> --- final.c (revision 248684)
> +++ final.c (working copy)
> @@ -1951,9 +1951,11 @@ dump_basic_block_info (FILE *file, rtx_i
> fprintf (file, "%s BLOCK %d", ASM_COMMENT_START, bb->index);
> if (bb->frequency)
> fprintf (file, " freq:%d", bb->frequency);
> - if (bb->count)
> - fprintf (file, " count:%" PRId64,
> - bb->count);
> + if (bb->count.initialized_p ())
> + {
> + fprintf (file, " count");
Missing colon.
s/count"/count:"/
> Index: profile-count.h
> ===================================================================
> --- profile-count.h (revision 0)
> +++ profile-count.h (working copy)
> +/* Main data type to hold profile counters in GCC. In most cases profile
> + counts originate from profile feedback. They are 64bit integers
> + representing number of executions during the train run.
> + As the profile is maintained during the compilation, many adjustments are
> + made. Not all transformations can be made precisely, most importantly
> + when code is being duplicated. It also may happen that part of CFG has
> + profile counts known while other does not - for example when LTO
> optimizing
s/does not/do not/;# i think
> + partly profiled program or when profile was lost due to COMDAT merging.
> +
> + For this information profile_count trakcs more information than
tracks
> +class GTY(()) profile_count
> +{
> + /* Value of counters which has not been intiailized. Either becuase
initialized
because
> + initializatoin did not happen yet or because profile is unknown. */
initialization
> + static profile_count uninitialized ()
> + {
> + profile_count c;
> + c.m_val = -1;
> + return c;
> + }
> +
> + /* The profiling runtime uses gcov_type, which is usually 64bit integer.
> + Conversins back and forth are used to read the coverage and get it
Conversions
> + profile_count &operator+= (const profile_count &other)
I think i saw a_count = a_count + something above and assumed you didn't
have a += operator. Could thus use the terse form in the snipped code
above on the patch, maybe?
> + /* Return *this * num / den. */
Parameter names in comment in caps please.
> Index: tree-tailcall.c
> ===================================================================
> --- tree-tailcall.c (revision 248684)
> +++ tree-tailcall.c (working copy)
> @@ -767,12 +767,10 @@ adjust_return_value (basic_block bb, tre
> /* Subtract COUNT and FREQUENCY from the basic block and it's
> outgoing edge. */
> static void
> -decrease_profile (basic_block bb, gcov_type count, int frequency)
> +decrease_profile (basic_block bb, profile_count count, int frequency)
> {
> edge e;
> - bb->count -= count;
> - if (bb->count < 0)
> - bb->count = 0;
> + bb->count = bb->count - count;
That's one of the spots where i'd have expected use of operator -=, fwiw.
> Index: value-prof.c
> ===================================================================
> --- value-prof.c (revision 248684)
> +++ value-prof.c (working copy)
> @@ -588,8 +588,10 @@ free_histograms (struct function *fn)
>
> static bool
> check_counter (gimple *stmt, const char * name,
> - gcov_type *count, gcov_type *all, gcov_type bb_count)
> + gcov_type *count, gcov_type *all, profile_count bb_count_d)
> {
> + gcov_type bb_count = bb_count_d.to_gcov_type ();
> + return true;
On purpose?
thanks,
