[PATCH] Add new verification for profile-count.h.
Tom de Vries
Tom_deVries@mentor.com
Sun Jan 14 10:31:00 GMT 2018
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
>>
>
More information about the Gcc-patches
mailing list