Re: [PATCH 1/2] profile-count: Handle signed values in profile_probability::apply

Since this changes profile-count perhaps Honza is best placed to look at it=
?
Adding to CC.
Thanks,
Kyrill

> On 31 Jul 2026, at 14:49, Kyrylo Tkachov <[email protected]> wrote:
>=20
> Ping.
> Thanks,
> Kyrill
>=20
>> On 24 Jul 2026, at 14:27, Kyrylo Tkachov <[email protected]> wrote:
>>=20
>> Ping.
>> Thanks,
>> Kyrill
>>=20
>>> On 17 Jul 2026, at 15:25, Kyrylo Tkachov <[email protected]> wrote:
>>>=20
>>> From: Kyrylo Tkachov <[email protected]>
>>>=20
>>> 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.
>>>=20
>>> Scale the unsigned magnitude with safe_scale_64bit, then restore the si=
gn.
>>> Return the input directly for a unit probability so that the minimum
>>> gcov_type value remains representable.  Preserve the existing truncatio=
n
>>> for uninitialized probabilities.
>>>=20
>>> Add selftests for signed rounding, zero and unit probabilities,
>>> uninitialized probabilities, and the minimum and maximum gcov_type valu=
es.
>>>=20
>>> Bootstrapped and tested on aarch64-linux-gnu and x86_64-linux.
>>> Ok for trunk?
>>>=20
>>> Thanks,
>>> Kyrill
>>>=20
>>> gcc/ChangeLog:
>>>=20
>>> * 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.
>>>=20
>>> 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(-)
>>>=20
>>> 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"
>>>=20
>>> /* Names from profile_quality enum values.  */
>>>=20
>>> @@ -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 =3D INTTYPE_MINIMUM (gcov_type);
>>> +  const gcov_type max =3D INTTYPE_MAXIMUM (gcov_type);
>>> +  profile_probability quarter =3D profile_probability::guessed_always =
() / 4;
>>> +  profile_probability even =3D 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 =3D profile_probability::uninitial=
ized ();
>>> +  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;
>>>   }
>>>=20
>>> +  /* Return VAL scaled by this probability.  Round initialized probabi=
lities
>>> +     to the nearest integer, with halfway values away from zero.  Trea=
t an
>>> +     uninitialized probability as one half and truncate toward zero.  =
*/
>>> gcov_type apply (gcov_type val) const
>>>   {
>>>     if (*this =3D=3D 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 =3D=3D 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
>>> + =3D val < 0 ? -(gcov_type_unsigned) val : (gcov_type_unsigned) val;
>>> +      uint64_t scaled;
>>> +      bool scaled_p
>>> + =3D safe_scale_64bit (magnitude, m_val, max_probability, &scaled);
>>> +      /* The scaled result fits in uint64_t.  With the unit case handl=
ed above,
>>> + it also fits in the nonnegative range of gcov_type.  */
>>> +      gcc_checking_assert (scaled_p);
>>> +      gcc_checking_assert
>>> + (scaled <=3D (gcov_type_unsigned) INTTYPE_MAXIMUM (gcov_type));
>>> +
>>> +      return val < 0 ? -(gcov_type) scaled : (gcov_type) scaled;
>>>   }
>>>=20
>>> /* 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 ();
>>> --=20
>>> 2.50.1 (Apple Git-155)
>>>=20
>>=20
>=20