On Mon, Aug 10, 2026 at 4:28 PM <[email protected]> wrote: > > 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-none-linux and x86_64-linux. > Ok for trunk?
So the reason I have not yet acked this is that I wonder why we'd have negative probabilities at all? To me that doesn't make sense. > 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 1d21b8751b4..07eb984e918 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 f2e2b49fc20..acfc4a46e49 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) >
