[PATCH] D48283: [SCEV] Properly solve quadratic equations

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Only one non-trivial question inline.



================
Comment at: unittests/ADT/APIntTest.cpp:2368
+
+    if (Solution == 0) {
+      EXPECT_EQ(C & Mask, 0);
----------------
Why do we need this special case?


================
Comment at: unittests/ADT/APIntTest.cpp:2382
+    auto IsZeroOrOverflow = [&] (int X) {
+      int64_t VX = A*X*X + B*X + C;
+      int64_t OverX = OverflowBits(VX, Width);
----------------
s/VX/ValueAtX


================
Comment at: unittests/ADT/APIntTest.cpp:2384
+      int64_t OverX = OverflowBits(VX, Width);
+      return (VX & Mask) == 0 || OverX != Over0;
+    };
----------------
I don't quite understand this.  Don't you need to check that the overflow bits are different from when the equation is evaluated at `X-1` (and not from when the equation is evaluated at `0`, which is what it looks like you're doing here)?


================
Comment at: unittests/ADT/APIntTest.cpp:2387
+
+    auto EqStr = [&] (const char *X_str) {
+      return Twine(A) + Twine(X_str) + Twine("^2 + ") + Twine(B) +
----------------
Probably better to call this `EquationToString`?


Repository:
  rL LLVM

https://reviews.llvm.org/D48283