This is an archive of the discontinued LLVM Phabricator instance.

[ConstantRange] Add urem support
ClosedPublic

Authored by nikic on Apr 21 2019, 1:04 PM.

Details

Summary

Add urem support to ConstantRange, so we can handle in in LVI. This is an approximate implementation that tries to capture the most useful conditions: If the LHS is always strictly smaller than the RHS, then the urem is a no-op and the result is the same as the LHS range. Otherwise the lower bound is zero and the upper bound is min(LHSMax, RHSMax - 1).

Diff Detail

Repository
rL LLVM

Event Timeline

nikic created this revision.Apr 21 2019, 1:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2019, 1:04 PM
nikic updated this revision to Diff 196016.Apr 21 2019, 2:04 PM

Make implementation slightly more obvious.

While i understand that you can't do a traditional exhaustive test that this produces *identical* results,
can't you at least test that the ConstantRange::urem() is still always conservatively correct?

lebedev.ri accepted this revision.Apr 23 2019, 7:50 AM

LG other than exhaustive test suggestion.

Even in the cases where we can't test that exhaustive test produces identical results,
we should be able to at least test that ConstantRange is conservatively correct,
in the sense that ConstantRange returns a superset of the exhaustive test, no?

This revision is now accepted and ready to land.Apr 23 2019, 7:50 AM
This revision was automatically updated to reflect the committed changes.

Even in the cases where we can't test that exhaustive test produces identical results,
we should be able to at least test that ConstantRange is conservatively correct,
in the sense that ConstantRange returns a superset of the exhaustive test, no?

Yup. I've extended TestUnsignedBinOpExhaustive to support checking correctness only and used it here.

Even in the cases where we can't test that exhaustive test produces identical results,
we should be able to at least test that ConstantRange is conservatively correct,
in the sense that ConstantRange returns a superset of the exhaustive test, no?

Yup. I've extended TestUnsignedBinOpExhaustive to support checking correctness only and used it here.

Great, thank you!