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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 24 15:34:55 PDT 2018


efriedma added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:8558
+/// solution fits in the original bit width, truncate it (except for i1).
+/// Returning a value of a different bit width may inhibit some optimizations.
+static Optional<APInt> TruncIfPossible(Optional<APInt> X, unsigned BitWidth) {
----------------
kparzysz wrote:
> efriedma wrote:
> > Is it possible for the truncation to fail?  For an AddRec of bitwidth N, evaluateAtIteration(2^N)==evaluateAtIteration(0), so any solution must be less than 2^N.  (And all users should treat the backedge-taken count as an unsigned quantity.)
> It is possible.  The test `test/Analysis/ScalarEvolution/solve-quadratic-i1.ll` has coefficients in i1, but the solution is 2 (the loop iterates 3 times).  I'm not sure if this is specific to i1 or not.  It hasn't happened in any test I've run outside of i1.
Oh, I'm just wrong; in general, the solution for an AddRec of bitwidth B needs up to B+1 bits.  Maybe worth noting that explicitly.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:8513
+  unsigned BitWidth = LC->getAPInt().getBitWidth();
+  unsigned NewWidth = BitWidth + 2;
+  LLVM_DEBUG(dbgs() << __func__ << ": addrec coeff bw: "
----------------
I think `BitWidth + 1` should be sufficient?  At least, that should be enough to convert the AddRec to an equation without losing any information.  (Like you note in the comments later, the equation is `L + nM + n(n-1)/2 N = 0 mod BitWidth`, or `2L + 2M n + n(n-1) N = 0 mod BitWidth+1`.)


================
Comment at: lib/Analysis/ScalarEvolution.cpp:8518
+  M = M.sext(NewWidth);
+  L = L.sext(NewWidth);
 
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D48283





More information about the llvm-commits mailing list