From: Kyrylo Tkachov <[email protected]>

profile_probability::apply uses RDIV, which adds half the denominator
before division.  This does not round negative values correctly.
Multiplying the signed input by the fixed-point probability can also
overflow for large inputs.

Scale the unsigned magnitude with safe_scale_64bit, then restore the sign.
Return the input directly for a unit probability so that the minimum
gcov_type value remains representable.  Preserve the existing truncation
for uninitialized probabilities.

Add selftests for signed rounding, zero and unit probabilities,
uninitialized probabilities, and the minimum and maximum gcov_type values.

Bootstrapped and tested on aarch64-linux-gnu and x86_64-linux.
Ok for trunk?

Thanks,
Kyrill

gcc/ChangeLog:

        * profile-count.h (profile_probability::apply): Scale an unsigned
        magnitude with safe_scale_64bit.
        * profile-count.cc (test_profile_probability_apply): New.
        (profile_count_cc_tests): New.
        * selftest.h (profile_count_cc_tests): Declare.
        * selftest-run-tests.cc (selftest::run_tests): Run it.

Signed-off-by: Kyrylo Tkachov <[email protected]>
---
 gcc/profile-count.cc      | 47 +++++++++++++++++++++++++++++++++++++++
 gcc/profile-count.h       | 26 +++++++++++++++++++++-
 gcc/selftest-run-tests.cc |  1 +
 gcc/selftest.h            |  1 +
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/gcc/profile-count.cc b/gcc/profile-count.cc
index 2cce9caa772..9be209efda2 100644
--- a/gcc/profile-count.cc
+++ b/gcc/profile-count.cc
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "wide-int.h"
 #include "sreal.h"
 #include "profile.h"
+#include "selftest.h"
 
 /* Names from profile_quality enum values.  */
 
@@ -595,3 +596,49 @@ profile_count::force_nonzero () const
     }
   return ret;
 }
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Verify profile_probability::apply.  */
+
+static void
+test_profile_probability_apply ()
+{
+  const gcov_type min = INTTYPE_MINIMUM (gcov_type);
+  const gcov_type max = INTTYPE_MAXIMUM (gcov_type);
+  profile_probability quarter = profile_probability::guessed_always () / 4;
+  profile_probability even = profile_probability::even ();
+
+  ASSERT_EQ (1, quarter.apply (3));
+  ASSERT_EQ (-1, quarter.apply (-3));
+  ASSERT_EQ (2, even.apply (3));
+  ASSERT_EQ (-2, even.apply (-3));
+  ASSERT_EQ (0, even.apply (0));
+
+  ASSERT_EQ (0, profile_probability::never ().apply (min));
+  ASSERT_EQ (min, profile_probability::always ().apply (min));
+  ASSERT_EQ (max, profile_probability::always ().apply (max));
+  ASSERT_EQ (min, profile_probability::guessed_always ().apply (min));
+  ASSERT_EQ (min / 2, even.apply (min));
+  ASSERT_EQ (max / 2 + 1, even.apply (max));
+
+  profile_probability uninitialized = profile_probability::uninitialized ();
+  ASSERT_EQ (1, uninitialized.apply (3));
+  ASSERT_EQ (-1, uninitialized.apply (-3));
+  ASSERT_EQ (min / 2, uninitialized.apply (min));
+  ASSERT_EQ (max / 2, uninitialized.apply (max));
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+profile_count_cc_tests ()
+{
+  test_profile_probability_apply ();
+}
+
+} // namespace selftest
+
+#endif
diff --git a/gcc/profile-count.h b/gcc/profile-count.h
index b424ecba3ea..76325d0fdfa 100644
--- a/gcc/profile-count.h
+++ b/gcc/profile-count.h
@@ -513,11 +513,35 @@ public:
       return ret;
     }
 
+  /* Return VAL scaled by this probability.  Round initialized probabilities
+     to the nearest integer, with halfway values away from zero.  Treat an
+     uninitialized probability as one half and truncate toward zero.  */
   gcov_type apply (gcov_type val) const
     {
       if (*this == uninitialized ())
        return val / 2;
-      return RDIV (val * m_val, max_probability);
+
+      /* A unit probability leaves VAL unchanged.  Return it directly because
+        the magnitude of the minimum gcov_type value is one greater than the
+        maximum gcov_type value.  */
+      if (m_val == max_probability)
+       return val;
+
+      /* Convert to unsigned before negating so that the minimum gcov_type
+        value has a representable magnitude.  Scale the magnitude with
+        overflow-safe arithmetic, then restore the sign.  */
+      gcov_type_unsigned magnitude
+       = val < 0 ? -(gcov_type_unsigned) val : (gcov_type_unsigned) val;
+      uint64_t scaled;
+      bool scaled_p
+       = safe_scale_64bit (magnitude, m_val, max_probability, &scaled);
+      /* The scaled result fits in uint64_t.  With the unit case handled above,
+        it also fits in the nonnegative range of gcov_type.  */
+      gcc_checking_assert (scaled_p);
+      gcc_checking_assert
+       (scaled <= (gcov_type_unsigned) INTTYPE_MAXIMUM (gcov_type));
+
+      return val < 0 ? -(gcov_type) scaled : (gcov_type) scaled;
     }
 
   /* Return 1-*THIS.  */
diff --git a/gcc/selftest-run-tests.cc b/gcc/selftest-run-tests.cc
index e39a94f8688..a281a239b10 100644
--- a/gcc/selftest-run-tests.cc
+++ b/gcc/selftest-run-tests.cc
@@ -72,6 +72,7 @@ selftest::run_tests ()
   wide_int_cc_tests ();
   ggc_tests_cc_tests ();
   sreal_cc_tests ();
+  profile_count_cc_tests ();
   fibonacci_heap_cc_tests ();
   typed_splay_tree_cc_tests ();
   opt_suggestions_cc_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 8891d0b7b6f..f9dacd69af7 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -248,6 +248,7 @@ extern void path_coverage_cc_tests ();
 extern void predict_cc_tests ();
 extern void pretty_print_cc_tests ();
 extern void pretty_print_token_buffer_cc_tests ();
+extern void profile_count_cc_tests ();
 extern void pub_sub_cc_tests ();
 extern void range_op_tests ();
 extern void range_tests ();
-- 
2.50.1 (Apple Git-155)

Reply via email to