This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] get a more accurate range for AddRecExpr with nsw flag
ClosedPublic

Authored by shchenz on Jan 8 2020, 10:34 PM.

Diff Detail

Event Timeline

shchenz created this revision.Jan 8 2020, 10:34 PM
nikic added inline comments.Jan 10 2020, 1:00 AM
llvm/lib/Analysis/ScalarEvolution.cpp
5688–5689

With the new code, we can skip operand zero (start) here. We only need that the step operands have a known sign so the addrec moves in a known direction, but the sign of the start value doesn't matter.

shchenz updated this revision to Diff 237322.Jan 10 2020, 7:41 AM
shchenz marked an inline comment as done.

operand 0 does not required to be NonPos/NonNeg

llvm/lib/Analysis/ScalarEvolution.cpp
5688–5689

Good point.

I'd like to see a few more testcases here, to cover the new functionality

nikic added a comment.Jan 10 2020, 9:48 AM

Logic looks good to me. As @efriedma mentioned some extra tests would be good, especially for the different sign cases. The current test only covers negative start with negative step.

llvm/lib/Analysis/ScalarEvolution.cpp
5702

It's possible to use getNonEmpty() to avoid the explicit check for signed-min / signed-max:

ConservativeResult = ConservativeResult.intersectWith(
    ConstantRange::getNonEmpty(getSignedRangeMin(AddRec->getStart()),
    APInt::getSignedMinValue(BitWidth)),
    RangeType);
shchenz updated this revision to Diff 237479.Jan 10 2020, 7:53 PM

use getNonEmpty() interface & add more test cases

nikic accepted this revision.Jan 11 2020, 1:42 AM

LGTM

llvm/lib/Analysis/ScalarEvolution.cpp
5704

This looks a bit ugly, it might make sense to insert a newline after getNonEmpty( instead and only do the 4-space indent, so there's still space for the + 1.

This revision is now accepted and ready to land.Jan 11 2020, 1:42 AM
shchenz marked an inline comment as done.Jan 11 2020, 8:19 PM
shchenz added inline comments.
llvm/lib/Analysis/ScalarEvolution.cpp
5704

Thanks, this is formatted by clang-format. I will reformat it manually.

This revision was automatically updated to reflect the committed changes.