This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] Favor isKnownViaSimpleReasoning over constant ranges check
ClosedPublic

Authored by mkazantsev on Feb 12 2018, 1:59 AM.

Details

Summary

There is a more powerful but still simple function isKnownViaSimpleReasoning that
does constant range check and few more additional checks. We use it some places (e.g.
when proving implications) and in some other places we only check constant ranges.

Currently, indvar simplifier fails to remove the check in following loop:

int inc = ...;
for (int i = inc, j = inc - 1; i < 200; ++i, ++j)
  if (i > j) { ... }

This patch replaces all usages of isKnownPredicateViaConstantRanges with
isKnownViaSimpleReasoning to have smarter proofs. In particular, it fixes the
case above.

Diff Detail

Repository
rL LLVM

Event Timeline

mkazantsev created this revision.Feb 12 2018, 1:59 AM
sanjoy accepted this revision.Feb 13 2018, 10:31 PM

lgtm.

You may want to indicate the range of inc in the commit message; otherwise it looks like we're introducing a miscompile. :)

lib/Analysis/ScalarEvolution.cpp
8962 ↗(On Diff #133813)

I think it is better to call this isKnownViaNonRecursiveReasoning since "simple" is not very descriptive (if you agree can you please directly land that in a subsequent change?).

test/Transforms/IndVarSimplify/loop-invariant-conditions.ll
144 ↗(On Diff #133813)

Let's move this to the bottom of the file.

This revision is now accepted and ready to land.Feb 13 2018, 10:31 PM
mkazantsev added inline comments.Feb 14 2018, 10:03 PM
lib/Analysis/ScalarEvolution.cpp
8962 ↗(On Diff #133813)

Will do as follow-up NFC.

test/Transforms/IndVarSimplify/loop-invariant-conditions.ll
144 ↗(On Diff #133813)

Will do as follow-up.

This revision was automatically updated to reflect the committed changes.