[PATCH] D27734: [LoopUnroll] Enable PGO-based loop peeling by default

Michael Kuperstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 13 15:30:46 PST 2017


mkuper added a comment.

In https://reviews.llvm.org/D27734#644780, @anemet wrote:

> > Other than that, the current heuristic doesn't distinguish between hot and cold loops. This is rather unfortunate, but I'm not sure how it can be fixed.
> >  Both because using BFI in a loop pass is problematic (hence llvm::getLoopEstimatedTripCount() - although it's also cheaper than computing full BFI just to get the equivalent), and because it's not clear that relative hotness to function entry is the right metric here. Maybe relative hotness to the entire profile?
>
> ProfileSummaryInfo should have this capability.


There are two problems with this:

1. ProfileSummaryInfo uses function entry counts to decide which functions are hot. But it doesn't give you information about the hotness of a block. To get the global hotness of a block (e.g. the loop header), you'd need to know both the relative hotness of a function (PSI) and the hotness of the loop header relative to the function entry (BFI).  If BFI is unavailable, you can assume branch probabilities aren't scaled, and correspond exactly to the number of times each branch was taken/not-taken, but this is fishy. It's "more or less fine" if you have BFI, but it's imprecise - but it sounds like a bad idea to rely on alone, when BFI isn't available.

(Querying BFI and then, if BFI thinks the block is cold, relying on the branch weights being "correct", is, in fact, exactly how  ProfileSummaryInfo::isHotBB() works.)

2. It's not clear what the thresholds should be. Again, ProfileSummaryInfo has thresholds for hot/cold functions, not blocks. I guess we could go with "only peel low trip-count loops within a hot function". - this will partially solve the problem.

> Anyhow I didn't mean to hold up this patch for this reason; this could be a further improvement on top of this.  LGTM.

Thanks!
I'm currently holding this myself, because of https://reviews.llvm.org/D28593.


https://reviews.llvm.org/D27734





More information about the llvm-commits mailing list