Honestly, this was what convinced me something was off here.
As I see it, we try to do two things with one number, and that works just fine at first:
- We use branch profiles for block frequency calculation. Follow the paths, multiply the probabilities, and accumulate your block frequency. Nice.
- We can read estimated loop trip counts from block frequencies (or branch profiles).
It is important to note that:
Block frequencies are, by design, a “path-oriented” metric that takes all paths from the entry to the block into account.
Estimated loop trip counts are “localized” metrics that only care about the loop (backedge/exit taken probabilities).
If the profiled CFG is unmodified, the difference doesn’t matter. However, once you modify the loop, you might start to inteleave the path to the loop with the “loop path” itself. That’s where the issues start.
The example shows nicely how the current logic tries to repair the estimated loop trip count after unrolling/peeling. It mostly works, but it will destroy the branch probability since the repair focused on the remainder loop, ignoring the iterations that became part of the path. If we remove the “hacks” to “repair” the estimated loop trip count, we would naturally preserve the branch probabilities (yay!). This is already a strong argument for me.
Again, no extra logic is required to preserve the branch probabilities in the unroll/peel case, and all the complicated special handling destroys this fundamental profile property.
Now, keeping branch probabilities alive is a win, but we would pessimize the estimated loop trip count if we rip out the special handling. To get the best of both worlds, I’d suggest recording the estimated loop trip count eagerly, that is, at profile ingestion time. We read the profile data, for all loops we add the estimated trip count, and all transformations that modify loop trip counts modify those.
This is mostly in line with the draft PR, IIRC.