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 <mli...@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  <mli...@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



Reply via email to