[PATCH] D28460: getLoopEstimatedTripCount should really be called getLoopEstimatedBackedgeTakeCount.

Xin Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 9 10:59:55 PST 2017


trentxintong added a comment.

In https://reviews.llvm.org/D28460#639939, @mkuper wrote:

> I don't think this is the change we want to make here, for a few reasons:
>
> 1. On the API level, I think it makes sense to have the estimated loop trip-count. So, if we want to add 1 to the BE count, that should probably just happen inside the function, instead of renaming the function but keeping functionality as is.


I fully agree.

> 2. I'm not sure there's an actual off-by-1 here. Or, rather, there's definitely an off-by-1, but only in some cases - I guess it depends on whether the loop is bottom-tested or not?

In case the loop is bottom tested (rotated), there would be an off-by-1, as the loop body is executed backedge-count + 1 times.
If its not bottom tested, we would not have this profile data on the latch, right ? I would assume the latch simply branches back to the header and the test is in the header.

> 3. I've actually verified that this gives the right results for instrumentation-based FDO for loops with a known trip count. It's possible that I simply made a mistake - or perhaps there's an error somewhere else that this cancels out with. If you want to change this, could you please check that again?




https://reviews.llvm.org/D28460





More information about the llvm-commits mailing list