[PATCH] D25963: [LoopUnroll] Implement profile-based loop peeling

[Michael Kuperstein via llvm-commits]

mkuper added a subscriber: bogner.
mkuper added a comment.

In https://reviews.llvm.org/D25963#596720, @anemet wrote:

> In https://reviews.llvm.org/D25963#596582, @mkuper wrote:
>
> > Thanks, Adam!
> >  I'll fix the cosmetics, but I'm still not sure what to do about the branch weight adjustments.
>
>
> Wasn't @davidxl working on fixing them?


Not that I know of. @davidxl, did I miss something?

Also, phab seems to have swallowed a long comment I wrote about that. :-\

Basically, the underlying issue is that clang doesn't actually record the branch weights it gets from the profile. Rather, it records the branch weights it thinks are better for the purpose of branch probability estimation.  >From CodeGenPGO.cpp:
/// According to Laplace's Rule of Succession, it is better to compute the
/// weight based on the count plus 1, so universally add 1 to the value.
While this may be good from a branch-probability standpoint, it throws off using the weights for trip count estimates, when the exit weight is small.

I see 3 options:

1. Keep what I have here as a band-aid.
2. Don't do the -1 adjustment here - hopefully, in reality, we don't care about the cases where it makes a difference. I'll need to benchmark this, though.
3. Remove the +1 adjustment in clang, and instead do it where we actually use the branch weights to estimate probabilities, that is, in BPI. Of course, there may be other places that read the weights but really want the adjusted weights (and, say, assume weights are never 0). I asked @bogner about this on IRC, and he seemed ambivalent.

What do you think?


https://reviews.llvm.org/D25963