Hi,
so what goes wrong with the testcase is the fact that after recursive
inliing we have large loop nest and consequently invalid profile since
every loop is predicted to iterate quite a lot. Rebuild_frequences
should take care of the problem, but it doesn't since there is:
if (freq_max < 16)
freq_max = 16;
Removing this check solves the testcase. Looking how it went in, I made
it in 2017 when dropping the original code to scale into range 0...10000
https://gcc.gnu.org/pipermail/gcc-patches/2017-November/488115.html
I have no recolleciton of inventing that check, but I suppose one can
argue that we do not want to scale most of CFG to 0 since the branch
prediciton is likely wrong and we do not know if the code with
unrealistic BB profile is important at all. So perhaps it is safer to
cap rather than scale most of function body to 0.
profile_count arithmetics is indeed supposed to be saturating, it is bad
I managed to miss the check for such a common operation as + :(
>
> 2024-03-27 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/112303
> * profile-count.h (profile_count::operator+): Perform
> addition in uint64_t variable and set m_val to MIN of that
> val and max_count.
> (profile_count::operator+=): Likewise.
> (profile_count::operator-=): Formatting fix.
These two changes as OK.
In apply_probability
> @@ -1127,7 +1129,9 @@ public:
> if (!initialized_p ())
> return uninitialized ();
> profile_count ret;
> - ret.m_val = RDIV (m_val * prob, REG_BR_PROB_BASE);
> + uint64_t tmp;
> + safe_scale_64bit (m_val, prob, REG_BR_PROB_BASE, &tmp);
> + ret.m_val = MIN (tmp, max_count);
This exists only for old code that still uses REG_BR_PROB_BAS integer.
Valid prob is always prob <= REG_BR_PROB_BASE :)
So we need safe_scale_64bit to watch overflow, but result does not need
MIN.
> ret.m_quality = MIN (m_quality, ADJUSTED);
> return ret;
> }
> @@ -1145,7 +1149,7 @@ public:
> uint64_t tmp;
> safe_scale_64bit (m_val, prob.m_val,
> profile_probability::max_probability,
> &tmp);
> - ret.m_val = tmp;
> + ret.m_val = MIN (tmp, max_count);
Same here, it is unnecesary to do MIN.
OK with this change.
Thanks for looking into this,
Honza
