This is an archive of the discontinued LLVM Phabricator instance.

[ConstantRange] Add saturating add/sub methods
ClosedPublic

Authored by nikic on Apr 21 2019, 4:57 AM.

Details

Summary

Add support for uadd_sat and friends to ConstantRange, so we can handle uadd.sat and friends in LVI. The implementation is forwarding to the corresponding APInt methods with appropriate bounds.

One thing worth pointing out here is that the handling of wrapping ranges is not maximally accurate. A simple example is that adding 0 to a wrapped range will return a full range, rather than the original wrapping range. The tests also only check that the non-wrapping envelope is correct and minimal.

Diff Detail

Repository
rL LLVM

Event Timeline

nikic created this revision.Apr 21 2019, 4:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2019, 4:57 AM
lebedev.ri requested changes to this revision.Apr 21 2019, 6:03 AM

Looks good in general!

Are you confident the test passes?
If yes, i think there may be some much bigger problem with that whole test approach.

llvm/lib/IR/ConstantRange.cpp
1148–1150 ↗(On Diff #195991)

Will need rebasing after D60946.

llvm/unittests/IR/ConstantRangeTest.cpp
1674–1675 ↗(On Diff #195991)

Yes, i can see how that pattern is tedious to write :)

1692 ↗(On Diff #195991)

Uhm, are you sure this is correct?
I'd expect this to be:

APInt Min = APInt::getSignedMaxValue(Bits);
APInt Max = APInt::getSigned*Min*Value(Bits);
This revision now requires changes to proceed.Apr 21 2019, 6:03 AM
nikic updated this revision to Diff 196001.Apr 21 2019, 6:22 AM

Fix test.

nikic marked an inline comment as done.Apr 21 2019, 6:26 AM

Are you confident the test passes?

Well, that's embarrassing. The two signed tests were indeed failing...

nikic updated this revision to Diff 196003.Apr 21 2019, 6:35 AM
nikic marked 2 inline comments as done.

Rebase over D60947.

lebedev.ri accepted this revision.Apr 21 2019, 6:36 AM

Are you confident the test passes?

The two signed tests were indeed failing...

Aha, at least that didn't happen to expose some bigger problem then.
LG now.

Well, that's embarrassing.

This revision is now accepted and ready to land.Apr 21 2019, 6:36 AM
This revision was automatically updated to reflect the committed changes.