[PATCH] D60946: [ConstantRange] Add saturating add/sub methods

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

Looks good in general!

Are you confident the test passes?
If yes, i think there may be some much bigger problem with that whole test approach.



================
Comment at: llvm/lib/IR/ConstantRange.cpp:1148-1150
+  if (NewU == NewL)
+    return getFull();
+  return ConstantRange(std::move(NewL), std::move(NewU));
----------------
Will need rebasing after D60946.


================
Comment at: llvm/unittests/IR/ConstantRangeTest.cpp:1674-1675
+    ++Max;
+    ConstantRange Expected = Min == Max ? ConstantRange::getFull(Bits)
+                                        : ConstantRange(Min, Max);
+    EXPECT_EQ(Expected, CR);
----------------
Yes, i can see how that pattern is tedious to write :)


================
Comment at: llvm/unittests/IR/ConstantRangeTest.cpp:1692
+    APInt Min = APInt::getSignedMaxValue(Bits);
+    APInt Max = APInt::getSignedMaxValue(Bits);
+    ForeachNumInConstantRange(CR1, [&](const APInt &N1) {
----------------
Uhm, are you sure this is correct?
I'd expect this to be:
```
    APInt Min = APInt::getSignedMaxValue(Bits);
    APInt Max = APInt::getSigned*Min*Value(Bits);
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60946