[PATCH] D67339: [ConstantRange] add helper function addWithNoWrap

[ChenZheng via Phabricator via llvm-commits]

shchenz marked 2 inline comments as done.
shchenz added inline comments.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:834
+    if (isWrappedSet() || Other.isWrappedSet())
+      return add(Other);
+    APInt LMin = getUnsignedMin(), LMax = getUnsignedMax();
----------------
nikic wrote:
> You can remove the check for isWrappedSet / isSignWrappedSet now. We're going to intersect with the `add()` result anyway, so this is just going to intersect with itself. Even if one of the ranges is wrapped, we might still get a better result here if the other isn't. E.g. something like `[10, MAX] + Wrapped` will still get you `[10, MAX]` as the result here, which is likely better than what the wrapped calculation will provide.
I am a little confuse about removing the check for isWrappedSet / isSignWrappedSet.
Assuming we have two unsigned ranges [10, 20), [200, 100), if we remove the check for wrap, we get [10, 20) as result.

But if we do the math, [10, 20) +nuw [200, 100) results in two disjoint ranges [10, 119) \/ [210, 0).
My previous is a little like `getPreferredRange`, we choose one range from above disjoint ranges based on preferred type.

If we use [10, 20), will it be too aggresive, since we use a smaller range?
If we use `add` result for wrap range, we get [210, 119) as result which is father range of "[10, 119) \/ [210, 0)", it is conservative but right?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67339/new/

https://reviews.llvm.org/D67339