[PATCH] D25732: Introduce ConstantRange.addWithNoSignedWrap

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 18 11:02:44 PDT 2016

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)`).


More information about the llvm-commits mailing list