[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