[PATCH] D102982: [LoopUnroll] Use smallest exact trip count from any exit

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D102982#2782407 <https://reviews.llvm.org/D102982#2782407>, @reames wrote:

> @nikic We're talking past each other here.
>
> Let me work up from definitions.  I think this will make my point easier to follow.
>
> An exact exit count for an loop exit is the iteration on which that loop exit *must* be taken.  We know that no previous iteration can exit via that exit.  A maximum exit count is any upper bound on an exact exit count.  The only requirement is that it be a valid upper bound.  In practice, we only consider constant upper bounds, but that's an implementation detail.  See getExitCount(BB) and computeExitLimit for details.  Note that we only compute exit counts for exits which dominate latch as we don't want to have to reason about exits being skipped on the potentially exiting iteration.
>
> An exit count for a loop (either exact or max) is simply a umin of all the exit counts which dominate the latch.  See getBackedgeTakenCount and computeBackedgeTakenCount.

Uff. Okay, this is where the dissonance is probably coming from. My problem here is that I don't consider "umin of exact exit counts" a meaningful operation. The valuable thing about an exact exit count vs a max exit count is that it is guaranteed not-taken before the exit count. This property gets lost as soon as you take the umin of multiple exits.

I do see now that this is the existing behavior of getBackedgeTakenCount(). However, I would like to argue that this behavior is not ideal, and we should change it. In particular, I'd like to ask this question: We have `getBackedgeTakenCount()` and `getSymbolicMaxBackedgeTakenCount()`. These methods are basically the same, but one takes a umin over all exact exit counts, while the other takes the umin over all max exit counts. When would be ever want to use the former rather than the latter? Isn't the latter //strictly// better? Both will only give you an upper bound on the BECount, but the one that works with max exit counts will be more accurate. Neither guarantee a lower bound (for the multi-exit case).

Personally, I think that `getBackedgeTakenCount()` should be changed to return the `SymbolicMax` BECount, and the Exact BECount notion should be either removed entirely (and instead Exact counts should be queried for a specific exit) or else it should only return a value for single exit loops, the same as getSmallConstantTripCount/getSmallConstantTripMultiple currently do.

> My whole point has been - aside from the need to know which exit has the smallest trip count - that you could simply use the generic implementation which works for multiple exit loops.  There's no need for SCEV to continue to use the form restricted to single exit loops.

(Though I think that the "aside from the need to know which exit has the smallest trip count" makes this inapplicable to the unrolling use-case, unless we want to modify the SCEV API to provide that information as well.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102982/new/

https://reviews.llvm.org/D102982