[3/4] Dump histograms only if present.

gcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* value-prof.c (dump_histogram_value): Print histogram values
	only if present.
---
 gcc/value-prof.c | 72 +++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 44 deletions(-)

> 
> gcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* value-prof.c (dump_histogram_value): Print histogram values
> 	only if present.

What is the point of having histogram value when there are no counters?

Honza
> ---
>  gcc/value-prof.c | 72 +++++++++++++++++++-----------------------------
>  1 file changed, 28 insertions(+), 44 deletions(-)
> 

> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
> index e893ca084c9..25b957d0c0a 100644
> --- a/gcc/value-prof.c
> +++ b/gcc/value-prof.c
> @@ -228,42 +228,38 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>    switch (hist->type)
>      {
>      case HIST_TYPE_INTERVAL:
> -      fprintf (dump_file, "Interval counter range %d -- %d",
> -	       hist->hdata.intvl.int_start,
> -	       (hist->hdata.intvl.int_start
> -	        + hist->hdata.intvl.steps - 1));
>        if (hist->hvalue.counters)
>  	{
> -	   unsigned int i;
> -	   fprintf (dump_file, " [");
> -           for (i = 0; i < hist->hdata.intvl.steps; i++)
> -	     fprintf (dump_file, " %d:%" PRId64,
> -		      hist->hdata.intvl.int_start + i,
> -		      (int64_t) hist->hvalue.counters[i]);
> -	   fprintf (dump_file, " ] outside range:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[i]);
> +	  fprintf (dump_file, "Interval counter range %d -- %d",
> +		   hist->hdata.intvl.int_start,
> +		   (hist->hdata.intvl.int_start
> +		    + hist->hdata.intvl.steps - 1));
> +
> +	  unsigned int i;
> +	  fprintf (dump_file, " [");
> +	  for (i = 0; i < hist->hdata.intvl.steps; i++)
> +	    fprintf (dump_file, " %d:%" PRId64,
> +		     hist->hdata.intvl.int_start + i,
> +		     (int64_t) hist->hvalue.counters[i]);
> +	  fprintf (dump_file, " ] outside range:%" PRId64 ".\n",
> +		   (int64_t) hist->hvalue.counters[i]);
>  	}
> -      fprintf (dump_file, ".\n");
>        break;
>  
>      case HIST_TYPE_POW2:
> -      fprintf (dump_file, "Pow2 counter ");
>        if (hist->hvalue.counters)
> -	{
> -	   fprintf (dump_file, "pow2:%" PRId64
> -		    " nonpow2:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[1],
> -		    (int64_t) hist->hvalue.counters[0]);
> -	}
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "Pow2 counter pow2:%" PRId64
> +		 " nonpow2:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[1],
> +		 (int64_t) hist->hvalue.counters[0]);
>        break;
>  
>      case HIST_TYPE_SINGLE_VALUE:
>      case HIST_TYPE_INDIR_CALL:
> -      fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
> -			   ? "Single values " : "Indirect call "));
>        if (hist->hvalue.counters)
>  	{
> +	  fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
> +			       ? "Single values " : "Indirect call "));
>  	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
>  	    {
>  	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
> @@ -272,40 +268,28 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>  	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
>  		fprintf (dump_file, ", ");
>  	    }
> +	  fprintf (dump_file, ".\n");
>  	}
> -      fprintf (dump_file, ".\n");
>        break;
>  
>      case HIST_TYPE_AVERAGE:
> -      fprintf (dump_file, "Average value ");
>        if (hist->hvalue.counters)
> -	{
> -	   fprintf (dump_file, "sum:%" PRId64
> -		    " times:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[0],
> -		    (int64_t) hist->hvalue.counters[1]);
> -	}
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "Average value sum:%" PRId64
> +		 " times:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[0],
> +		 (int64_t) hist->hvalue.counters[1]);
>        break;
>  
>      case HIST_TYPE_IOR:
> -      fprintf (dump_file, "IOR value ");
>        if (hist->hvalue.counters)
> -	{
> -	   fprintf (dump_file, "ior:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[0]);
> -	}
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "IOR value ior:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[0]);
>        break;
>  
>      case HIST_TYPE_TIME_PROFILE:
> -      fprintf (dump_file, "Time profile ");
>        if (hist->hvalue.counters)
> -      {
> -        fprintf (dump_file, "time:%" PRId64,
> -                 (int64_t) hist->hvalue.counters[0]);
> -      }
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "Time profile time:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[0]);
>        break;
>      case HIST_TYPE_MAX:
>        gcc_unreachable ();

On 6/6/19 2:16 PM, Jan Hubicka wrote:
> What is the point of having histogram value when there are no counters?

Because we first create histograms in branch_prob -> gimple_find_values_to_profile and
later we read profile from file. At that point we know which are empty, but we don't
remove them.

Martin

> On 6/6/19 2:16 PM, Jan Hubicka wrote:
> > What is the point of having histogram value when there are no counters?
> 
> Because we first create histograms in branch_prob -> gimple_find_values_to_profile and
> later we read profile from file. At that point we know which are empty, but we don't
> remove them.

OK, so it is about the dump not crahsing when you do it in meantime.
That makes sense :)
So patch is OK.

Honza