This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] Teach SCEV to find maxBECount when loop endbound is variant
ClosedPublic

Authored by anna on Oct 11 2017, 2:43 PM.

Details

Summary

This patch teaches SCEV to calculate the maxBECount when the end bound
of the loop can vary. Note that we cannot calculate the exactBECount.

This will only be done when both conditions are satisfied:

  1. the loop termination condition is strictly LT.
  2. the IV is proven to not overflow.

This provides more information to users of SCEV and can be used to
improve identification of finite loops.

Event Timeline

anna created this revision.Oct 11 2017, 2:43 PM
mkazantsev requested changes to this revision.Oct 11 2017, 11:09 PM
mkazantsev added inline comments.
lib/Analysis/ScalarEvolution.cpp
9693

If the stride is known negative and IsSigned is true, can we return computeMaxBECount(End, -Stride, Start, BitWidth, IsSigned) instead of conservative estimate?

9694

Do we have a guarantee that stride is not zero? I think we should bail early unless we proved it or assert on that.

9710

Please add name for the last parameter in /* */

9808

Do we have a guarantee that the IV increment has no wrap flag set? Otherwise I don't believe it works for stride other than 1. For example:

L = load volatile ...
for (int i = 0; i < L; i += 2) {
  ...
  L = load volatile ...
}

If L is always SINT_MAX, this loop is infinite (if IV increment doesn't have nsw flag set).

For stride = 1 I think it should work fine even without nsw: we always stop when i = SINT_MAX at most, but for other step values we can leap across it.

test/Analysis/ScalarEvolution/max-trip-count.ll
345

Please add a test that if iv.next does not have nsw flag for this example then the max backage-taken count is unpredictable (this loop is infinite if start = 0, n is always SINT_MAX) because we leap across SINT_MAX.

This revision now requires changes to proceed.Oct 11 2017, 11:09 PM
anna added inline comments.Oct 12 2017, 5:17 AM
lib/Analysis/ScalarEvolution.cpp
9693

This code is never used for zero value stride or isKnownNegative. Please see line 9787 where we bail out on zero value and negative strides.

The computeMaxBE code is an NFC refactoring of existing maxBECount calculation when exactBECount is not known. This patch just uses the existing code for variant RHS as well, and relies on already existing overflow checks.

9694

yes, as mentioned in response above. I'll add an assert.

9808

All the overflow tests are already done above and it handles the case you mention: stride > 1, stride is not positive etc. For clarity, that's the comment added just above this code at line 9806.

Yes, for stride =1, we are fine without nsw because of the strictly LT condition. I've also added a testcase for that.

I will add a testcase for stride > 1, without nsw flag and confirm what I've said is true.

anna updated this revision to Diff 118780.Oct 12 2017, 6:03 AM
anna edited edge metadata.
anna marked 3 inline comments as done.

Addressed reviewer comments: added more tests regarding overflow, stride negative, stride unknown.
Updated a failing test where we now have more granular max BECount taken
value (This test update was missed in the original diff).

mkazantsev accepted this revision.Oct 13 2017, 1:21 AM

LGTM

lib/Analysis/ScalarEvolution.cpp
9701

Maybe make it

MaxValue = IsSigned ? APInt::getSignedMaxValue(BitWidth) : APInt::getMaxValue(BitWidth);
Limit = MaxValue - (StrideForMaxBECount - 1);

to reduce code duplication? Just a suggestion.

This revision is now accepted and ready to land.Oct 13 2017, 1:21 AM
This revision was automatically updated to reflect the committed changes.
anna marked an inline comment as done.Oct 13 2017, 7:32 AM
sanjoy added inline comments.Oct 13 2017, 9:59 AM
llvm/trunk/lib/Analysis/ScalarEvolution.cpp
9692 ↗(On Diff #118908)

This logic is specific to {Start,+,Stride} SLT End, right? If so, please name it in a way to make that obvious and mention that in the comment.

anna added inline comments.Oct 13 2017, 10:23 AM
llvm/trunk/lib/Analysis/ScalarEvolution.cpp
9692 ↗(On Diff #118908)

The logic is specific to {Start,+,Stride} LT End, it can be s< or u<.

I'll change it to computeMaxBECountForLT and add a comment about the type of addrec and termination condition that's supported.

anna marked 2 inline comments as done.Oct 16 2017, 10:47 AM