[gcc-11,backport] gcov-profile: Allow negative counts of indirect calls [PR105282]

Message ID 20220419192805.1328115-1-slyich@gmail.com
State New
Headers
Series [gcc-11,backport] gcov-profile: Allow negative counts of indirect calls [PR105282] |

Commit Message

Sergei Trofimovich April 19, 2022, 7:28 p.m. UTC
  From: Sergei Trofimovich <siarheit@google.com>

TOPN metrics are histograms that contain overall count and per-bucket
count. Overall count can be negative when two profiles merge and some
of per-bucket metrics are disacarded.

Noticed as an ICE on python PGO build where gcc crashes as:

    during IPA pass: modref
    a.c:36:1: ICE: in stream_out_histogram_value, at value-prof.cc:340
       36 | }
          | ^
    stream_out_histogram_value(output_block*, histogram_value_t*)
            gcc/value-prof.cc:340

gcc/ChangeLog:

	PR gcov-profile/105282
	* value-prof.cc (stream_out_histogram_value): Allow negative counts
	on HIST_TYPE_INDIR_CALL.

(cherry picked from commit 90a29845bfe7d6002e6c2fd49a97820b00fbc4a3)
---
 gcc/value-prof.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Martin Liška April 19, 2022, 7:32 p.m. UTC | #1
On 4/19/22 21:28, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <siarheit@google.com>
> 
> TOPN metrics are histograms that contain overall count and per-bucket
> count. Overall count can be negative when two profiles merge and some
> of per-bucket metrics are disacarded.

I'm fine with that but I think, as we're close to 11.3.0, it's up to release managers
who should approve that.

Cheers,
Martin

> 
> Noticed as an ICE on python PGO build where gcc crashes as:
> 
>      during IPA pass: modref
>      a.c:36:1: ICE: in stream_out_histogram_value, at value-prof.cc:340
>         36 | }
>            | ^
>      stream_out_histogram_value(output_block*, histogram_value_t*)
>              gcc/value-prof.cc:340
> 
> gcc/ChangeLog:
> 
> 	PR gcov-profile/105282
> 	* value-prof.cc (stream_out_histogram_value): Allow negative counts
> 	on HIST_TYPE_INDIR_CALL.
> 
> (cherry picked from commit 90a29845bfe7d6002e6c2fd49a97820b00fbc4a3)
> ---
>   gcc/value-prof.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
> index 42748771192..688089b04d2 100644
> --- a/gcc/value-prof.c
> +++ b/gcc/value-prof.c
> @@ -336,6 +336,10 @@ stream_out_histogram_value (struct output_block *ob, histogram_value hist)
>   	/* Note that the IOR counter tracks pointer values and these can have
>   	   sign bit set.  */
>   	;
> +      else if (hist->type == HIST_TYPE_INDIR_CALL && i == 0)
> +	/* 'all' counter overflow is stored as a negative value. Individual
> +	   counters and values are expected to be non-negative.  */
> +	;
>         else
>   	gcc_assert (value >= 0);
>
  

Patch

diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index 42748771192..688089b04d2 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -336,6 +336,10 @@  stream_out_histogram_value (struct output_block *ob, histogram_value hist)
 	/* Note that the IOR counter tracks pointer values and these can have
 	   sign bit set.  */
 	;
+      else if (hist->type == HIST_TYPE_INDIR_CALL && i == 0)
+	/* 'all' counter overflow is stored as a negative value. Individual
+	   counters and values are expected to be non-negative.  */
+	;
       else
 	gcc_assert (value >= 0);