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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:8518
+  M = M.sext(NewWidth);
+  L = L.sext(NewWidth);
 
----------------
kparzysz wrote:
> efriedma wrote:
> > Do we prefer to sign-extend just so abs(A) is small in common cases?  I think the choice of sign-extension vs zero-extension is arbitrary otherwise. Probably worth explicitly noting with a comment.
> Are you talking only about this?  There is another extension in `SolveQuadraticEquation` (at line 8325). Originally there was only one sign-extension, but after reorganizing this code, it has been split into two. I kept them identical to the original one (i.e. both as sign-extension).
> 
> In general the choice of sign-extension was actually to avoid "false positives" when checking for overflow.  For example, take q(x) = (x+1)(x-2) = x^2 - x - 2, in i3.  The zeros are -1 and 2, q(0) = q(1) = -2 (i.e. nothing exceeding the signed range of i3), so we can expect the calculated solution to be 2. With zero-extended values, the function would be r(x) = x^2 + 7x + 6 with both solutions negative: -6 and -1. The equation r(x) = 0 would then become r(x) = 8 to solve for an unsigned overflow, i.e. x^2 + 7x - 2 = 0. The root of that is approx. 0.27, so the returned solution would be 1.
I'm talking about specifically this one: the extension here will still be necessary even if we add an exact modular quadratic SCEV solver.

I guess the answer is essentially the same, but deserves its own comment.


Repository:
  rL LLVM

https://reviews.llvm.org/D48283