[PATCH] D70043: [ConstantRange] Add `mulWithNoWrap()` method

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Let me know if my explanation makes sense.



================
Comment at: llvm/lib/IR/ConstantRange.cpp:1022
+
+    if (NoWrapKind & OBO::NoSignedWrap) {
+      (void)getUnsignedMin().smul_ov(Other.getUnsignedMin(), Overflow);
----------------
lebedev.ri wrote:
> nikic wrote:
> > Why do we need to specially handle the nuw+nsw combination here?
> Even after fixing `OBO::NoSignedWrap` check this remains.
> Here in `OBO::NoUnsignedWrap` we did check that
> `getUnsignedMin() * Other.getUnsignedMin()`
> does not exhibit *unsigned* overflow,
> but we did not do any such check in `OBO::NoSignedWrap`.
> 
> There, even if `UnsignedMin` happens to be one of the values closest to 0,
> since we check that all values overflow, unless said range only contains
> a single `UnsignedMin`, we wouldn't discard such range as always-overflowing.
> 
> So i **think** this is correct.
> At least until precision tests say otherwise.
> 
Example:
`Bits 4 CR1 [2,0) CR2 [4,0) CR [-8,0)`

This doesn't always overflow in unsigned domain - i4 2 * i4 4 == i4 -8 (signed overflow)
This doesn't always overflow in signed domain - both ranges contain -1 value - i4 -1 * i4 -1 == i4 1 (unsigned overflow)

But there is no such value pair that exhibits both no signed overflow and no unsigned overflow at the same time.

Alive proof: https://rise4fun.com/Alive/1aK


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70043