[PATCH] D25732: Introduce ConstantRange.addWithNoSignedWrap

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

I think the algorithm in `ConstantRange::addWithNoSignedWrap` can be improved (see inline), but what you have here looks correct and most likely precise enough for the cases people will care about.

So I'd say let's move ahead with what's here and improve the implementation `ConstantRange::addWithNoSignedWrap` in parallel with working on the patches that will use this `addWithNoSignedWrap` interface.



================
Comment at: lib/IR/ConstantRange.cpp:682
+  // passing a single element range.
+  ConstantRange NSWRange = ConstantRange::makeGuaranteedNoWrapRegion(
+                                      BinaryOperator::Add,
----------------
Nice trick!

Nit: I'd use `auto` here.

However, this isn't precise for the following case (in `i8`):

```
Other = 20
*this = [50, -50)
```

Then I think your algorithm will have

```
NSWRange = [128, 108)
NSWConstrainedRange = full set // I've not verified this
Result = full set
```

However, we could have returned `[70, -30)` (naively, without considering the nsw) or `[-108, 128)`, both of which are more precise.

I'm now wondering if the best algorithm is actually to first add `Other` to `*this`  and then subtract out the portion that could not have resulted from an overflowing add (e.g. adding `5` rules out the result from containing `[INT_MIN, INT_MIN + 5)`).



https://reviews.llvm.org/D25732