[PATCH] D59193: [ConstantRange] Add overflow check helpers

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 11 12:47:53 PDT 2019


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


================
Comment at: llvm/unittests/IR/ConstantRangeTest.cpp:1317-1329
+          APInt N1(Bits, Lo1);
+          for (unsigned I1 = 0; I1 < Size1; I1++, N1++) {
+            APInt N2(Bits, Lo2);
+            for (unsigned I2 = 0; I2 < Size2; I2++, N2++) {
+              assert(CR1.contains(N1));
+              assert(CR2.contains(N2));
+
----------------
lebedev.ri wrote:
> Hm, so this is iterating so that `N1` is every point in `CR1` and `N2` is every point in `CR2`.
> Took me a moment to get that.
> Can this perhaps be rewritten with for-loop somehow, more explicitly?
> ```
> for(APInt N1 = CR1.getLower(), APInt N1Max = CR1.getUpper(); N1 != N1Max; N1++) {
>   for(APInt N2 = CR2.getLower(), APInt N2Max = CR2.getUpper(); N2 != N2Max; N2++) {
>     assert(CR1.contains(N1));
>     assert(CR2.contains(N2));
> 
>     if (OverflowFn(N1, N2))
>       HasOverflow = true;
>     else
>        HasNoOverflow = true;
>   }
> }
> ```
A simple for loop will not work for the case of a full range (as N1 != N1Max will fail immediately). That's why I'm using the current code based on the range size. I've added a comment to that effect.


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

https://reviews.llvm.org/D59193





More information about the llvm-commits mailing list