Add new verification for profile-count.h.

Hi.

Following patch adds new sanitization checks for profile_quality.
Problem is that zero initialization of a struct with profile_count will
lead to an invalid counter. This can help to catch them.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

> Hi.
> 
> Following patch adds new sanitization checks for profile_quality.
> Problem is that zero initialization of a struct with profile_count will
> lead to an invalid counter. This can help to catch them.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
OK,
thanks!
Honza
> Martin

> >From edec114cf1dd29bb571855a80e1b45ae040da200 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 10 Jan 2018 14:46:08 +0100
> Subject: [PATCH] Add new verification for profile-count.h.
> 
> gcc/ChangeLog:
> 
> 2018-01-12  Martin Liska  <mliska@suse.cz>
> 
> 	* profile-count.h (enum profile_quality): Use 0 as invalid
> 	enum value of profile_quality.
> ---
>  gcc/profile-count.h | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/profile-count.h b/gcc/profile-count.h
> index 3c5f720ee81..7a43917ebbc 100644
> --- a/gcc/profile-count.h
> +++ b/gcc/profile-count.h
> @@ -30,27 +30,27 @@ enum profile_quality {
>       or may not match reality.  It is local to function and can not be compared
>       inter-procedurally.  Never used by probabilities (they are always local).
>     */
> -  profile_guessed_local = 0,
> +  profile_guessed_local = 1,
>    /* Profile was read by feedback and was 0, we used local heuristics to guess
>       better.  This is the case of functions not run in profile fedback.
>       Never used by probabilities.  */
> -  profile_guessed_global0 = 1,
> +  profile_guessed_global0 = 2,
>  
>    /* Same as profile_guessed_global0 but global count is adjusted 0.  */
> -  profile_guessed_global0adjusted = 2,
> +  profile_guessed_global0adjusted = 3,
>  
>    /* Profile is based on static branch prediction heuristics.  It may or may
>       not reflect the reality but it can be compared interprocedurally
>       (for example, we inlined function w/o profile feedback into function
>        with feedback and propagated from that).
>       Never used by probablities.  */
> -  profile_guessed = 3,
> +  profile_guessed = 4,
>    /* Profile was determined by autofdo.  */
> -  profile_afdo = 4,
> +  profile_afdo = 5,
>    /* Profile was originally based on feedback but it was adjusted
>       by code duplicating optimization.  It may not precisely reflect the
>       particular code path.  */
> -  profile_adjusted = 5,
> +  profile_adjusted = 6,
>    /* Profile was read from profile feedback or determined by accurate static
>       method.  */
>    profile_precise = 7
> @@ -505,6 +505,8 @@ public:
>    /* Return false if profile_probability is bogus.  */
>    bool verify () const
>      {
> +      gcc_checking_assert (profile_guessed_local <= m_quality
> +			   && m_quality <= profile_precise);
>        if (m_val == uninitialized_probability)
>  	return m_quality == profile_guessed;
>        else if (m_quality < profile_guessed)
> @@ -784,6 +786,8 @@ public:
>    /* Return false if profile_count is bogus.  */
>    bool verify () const
>      {
> +      gcc_checking_assert (profile_guessed_local <= m_quality
> +			   && m_quality <= profile_precise);
>        return m_val != uninitialized_count || m_quality == profile_guessed_local;
>      }
>  
> -- 
> 2.14.3
>

On 01/12/2018 09:44 AM, Jan Hubicka wrote:
>> Hi.
>>
>> Following patch adds new sanitization checks for profile_quality.
>> Problem is that zero initialization of a struct with profile_count will
>> lead to an invalid counter. This can help to catch them.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
> OK,
> thanks!
> Honza
>> Martin
> 
>> >From edec114cf1dd29bb571855a80e1b45ae040da200 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Wed, 10 Jan 2018 14:46:08 +0100
>> Subject: [PATCH] Add new verification for profile-count.h.
>>
>> gcc/ChangeLog:
>>
>> 2018-01-12  Martin Liska  <mliska@suse.cz>
>>
>> 	* profile-count.h (enum profile_quality): Use 0 as invalid
>> 	enum value of profile_quality.
>> ---
>>   gcc/profile-count.h | 16 ++++++++++------
>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/gcc/profile-count.h b/gcc/profile-count.h
>> index 3c5f720ee81..7a43917ebbc 100644
>> --- a/gcc/profile-count.h
>> +++ b/gcc/profile-count.h
>> @@ -30,27 +30,27 @@ enum profile_quality {
>>        or may not match reality.  It is local to function and can not be compared
>>        inter-procedurally.  Never used by probabilities (they are always local).
>>      */
>> -  profile_guessed_local = 0,
>> +  profile_guessed_local = 1,
>>     /* Profile was read by feedback and was 0, we used local heuristics to guess
>>        better.  This is the case of functions not run in profile fedback.
>>        Never used by probabilities.  */
>> -  profile_guessed_global0 = 1,
>> +  profile_guessed_global0 = 2,
>>   
>>     /* Same as profile_guessed_global0 but global count is adjusted 0.  */
>> -  profile_guessed_global0adjusted = 2,
>> +  profile_guessed_global0adjusted = 3,
>>   
>>     /* Profile is based on static branch prediction heuristics.  It may or may
>>        not reflect the reality but it can be compared interprocedurally
>>        (for example, we inlined function w/o profile feedback into function
>>         with feedback and propagated from that).
>>        Never used by probablities.  */
>> -  profile_guessed = 3,
>> +  profile_guessed = 4,
>>     /* Profile was determined by autofdo.  */
>> -  profile_afdo = 4,
>> +  profile_afdo = 5,
>>     /* Profile was originally based on feedback but it was adjusted
>>        by code duplicating optimization.  It may not precisely reflect the
>>        particular code path.  */
>> -  profile_adjusted = 5,
>> +  profile_adjusted = 6,
>>     /* Profile was read from profile feedback or determined by accurate static
>>        method.  */
>>     profile_precise = 7
>> @@ -505,6 +505,8 @@ public:
>>     /* Return false if profile_probability is bogus.  */
>>     bool verify () const
>>       {
>> +      gcc_checking_assert (profile_guessed_local <= m_quality
>> +			   && m_quality <= profile_precise);

Hi,

FYI, in a no-bootstrap build, I'm seeing a lot of new warnings like this:
...
../../src/gcc/profile-count.h: In member function ‘bool 
profile_probability::verify() const’:
../../src/gcc/profile-count.h:509:20: warning: comparison is always true 
due to limited range of data type [-Wtype-limits]
        && m_quality <= profile_precise);
                     ^
../../src/gcc/system.h:742:14: note: in definition of macro ‘gcc_assert’
     ((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 
: 0))
               ^
../../src/gcc/profile-count.h:508:7: note: in expansion of macro 
‘gcc_checking_assert’
        gcc_checking_assert (profile_guessed_local <= m_quality
...

Indeed, profile_precise is 7 and m_quality is a 3 bits wide bitfield.

Thanks,
- Tom

>>         if (m_val == uninitialized_probability)
>>   	return m_quality == profile_guessed;
>>         else if (m_quality < profile_guessed)
>> @@ -784,6 +786,8 @@ public:
>>     /* Return false if profile_count is bogus.  */
>>     bool verify () const
>>       {
>> +      gcc_checking_assert (profile_guessed_local <= m_quality
>> +			   && m_quality <= profile_precise);
>>         return m_val != uninitialized_count || m_quality == profile_guessed_local;
>>       }
>>   
>> -- 
>> 2.14.3
>>
>