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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D70043#1771089 <https://reviews.llvm.org/D70043#1771089>, @nikic wrote:

> > Baseline ConstantRange::mul() does not have an exhaustive test,
> >  and if i add one (TestUnsignedBinOpExhaustive()+/*CorrectnessOnly=*/true),
> >  it doesn't pass.
>
> For my own sanity: This doesn't mean that the multiply() implementation is incorrect, just that this testing function is a bit overly aggressive in CorrectnessOnly mode. It expects the result to be a superset of the minimal unsigned bounding range.


Sure, of course, otherwise the `EXPECT_TRUE(CR.contains(N));` would complain.

> We might want to change the function to actually only check correctness via `EXPECT_TRUE(CR.contains(N))`.

Note that we already do that check, otherwise we wouldn't know that the result is at least conservatively correct.

> Looking a bit into what causes the failures for mulWithNoWrap when testing not only correctness for the unsigned case. I think there are two main causes:
> 
> - Something like `[0, 1] * [-8, -5]` produces `[-8, 0]`, while the testing code expects `[0, -5]`. This actual result is consistent with the "smallest range" preferred range mode, so this is just a testing deficiency.
> - Something like `[0, 2] * [-8, -5]` produces full-set, while the testing code expects `[0, -5]`. The reason here is that the result can be in the ranges `[0, 0]`, `[-8, -5]` or `[2 * -8, 2 * -5]`, where the last range is unsigned (and signed) wrapping, so the result could actually be the same as for the previous case. However, we don't recognize that the `2 * [-8, -5]` case is fully excluded.
> 
>   So I guess it would be pretty hard to make this work non-approximately...




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70043