Page MenuHomePhabricator

[ConstantRange] Split makeICmpRegion.
ClosedPublic

Authored by sanjoy on Mar 15 2015, 2:23 AM.

Details

Summary

This change splits makeICmpRegion into makeAllowedICmpRegion and makeSatisfyingICmpRegion with slightly different contracts. The first one is useful for determining what values some expression may take, while the second one is useful for determining what values are guaranteed to satisfy a given icmp.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy updated this revision to Diff 21994.Mar 15 2015, 2:23 AM
sanjoy retitled this revision from to [ConstantRange] Add an exact version of makeICmpRegion..
sanjoy updated this object.
sanjoy edited the test plan for this revision. (Show Details)
sanjoy added a reviewer: nlewycky.
sanjoy added a subscriber: Unknown Object (MLST).
sanjoy updated this revision to Diff 22069.Mar 16 2015, 9:13 PM

Address Nick's comments: split make makeICmpRegion into
makeAllowedICmpRegion and makeSatisfyingICmpRegion with clearly
specified contracts.

sanjoy retitled this revision from [ConstantRange] Add an exact version of makeICmpRegion. to [ConstantRange] Split makeICmpRegion..Mar 16 2015, 9:16 PM
sanjoy updated this object.
nlewycky added inline comments.Mar 17 2015, 4:47 PM
include/llvm/IR/ConstantRange.h
64 ↗(On Diff #22069)

Optional suggestion: ... predicate with *any* value contained within Other is contained *in* the ...

70 ↗(On Diff #22069)

Result = [0, 4) right? Or Pred = ule, either way?

lib/IR/ConstantRange.cpp
122 ↗(On Diff #22069)

Optional: I'd leave out the "== ~~(A intersect B)" part.

sanjoy added inline comments.Mar 17 2015, 4:50 PM
include/llvm/IR/ConstantRange.h
64 ↗(On Diff #22069)

Will fix.

70 ↗(On Diff #22069)

Will fix, thanks for catching!

lib/IR/ConstantRange.cpp
122 ↗(On Diff #22069)

Will fix.

sanjoy updated this revision to Diff 22146.Mar 17 2015, 5:09 PM
  • Address Nick's review.
nlewycky accepted this revision.Mar 17 2015, 5:11 PM
nlewycky edited edge metadata.

LGTM

This revision is now accepted and ready to land.Mar 17 2015, 5:11 PM
This revision was automatically updated to reflect the committed changes.