This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Compute range for abs without nsw
ClosedPublic

Authored by nikic on Mar 19 2019, 2:19 PM.

Details

Summary

This is a small followup to D59511. The code that was moved into computeConstantRange() there is a bit overly conversative: If the abs is not nsw, it does not compute any range. However, abs without nsw still has a well-defined contiguous unsigned range from 0 to SIGNED_MIN (inclusive). This is a lot less useful than the usual 0 to SIGNED_MAX (inclusive) range, but if we're already here we might as well specify it...

Diff Detail

Repository
rL LLVM

Event Timeline

nikic created this revision.Mar 19 2019, 2:19 PM
lebedev.ri added inline comments.Mar 19 2019, 2:25 PM
llvm/test/Transforms/InstSimplify/icmp-abs-nabs.ll
158 ↗(On Diff #191394)

Hm, after thinking about it a bit more, i think the test coverage could be improved.
In reality, this test only shows that we end up folding this pattern to true.
But what are the thresholds for that opt to happen?
I.e. i think you'd want to add two more test cases here, with 2147483649+1 and 2147483649-1 constants.

nikic marked an inline comment as done.Mar 19 2019, 2:31 PM
nikic added inline comments.
llvm/test/Transforms/InstSimplify/icmp-abs-nabs.ll
158 ↗(On Diff #191394)

The test directly below this one checks the <2147483648 case, which does not fold. So if <2147483648 does not fold but <2147483649 does, this has to be the upper boundary of the range. (It does not check the lower boundary, but as it's zero and >= 0 in unsigned domain is tautological we can't do a direct check for it.)

lebedev.ri accepted this revision.Mar 19 2019, 2:48 PM

Actually, i haven't fully noticed that there was that second test.
Looks good then modulo comment nit.

llvm/lib/Analysis/ValueTracking.cpp
5677–5678 ↗(On Diff #191394)

This comment reads weird.
I think you want something more like:

// If the negation part of the abs (in RHS) has NSW flag,
// then the result of abs(X) is [0..SIGNED_MAX],
// else it is [0..-SIGNED_MIN],

?

llvm/test/Transforms/InstSimplify/icmp-abs-nabs.ll
158 ↗(On Diff #191394)
This revision is now accepted and ready to land.Mar 19 2019, 2:48 PM
This revision was automatically updated to reflect the committed changes.