[PATCH] D48283: [SCEV] Properly solve quadratic equations
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 16 18:13:27 PDT 2018
efriedma added inline comments.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:8256
+/// Find the smallest non-negative number n, such that after n iterations
+/// the quadratic chrec {L,+,M,+,N} equals 0 or changes sign.
+static Optional<const SCEVConstant *>
----------------
Please clarify what happens if the addrec has signed overflow. Please document the conditions under which the algorithm fails to find a solution.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:8285
+ // In i1, 1 is the same as -1, so sign-extending (i1 1) would actually
+ // produce -1 in the larger type.
+ N = N.zext(NewWidth);
----------------
The algorithm should still work even if you treat "i1 true" as -1, I think. Actually, you end up solving the exact same equation, because of the `A.isNegative()` check.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:10632
+ if (Range.contains(RootVal->getValue()))
return SE.getCouldNotCompute(); // Something strange happened
----------------
Please make sure we have a testcase that hits this "getCouldNotCompute".
================
Comment at: lib/Analysis/ScalarEvolution.cpp:10641
+ return R;
return SE.getCouldNotCompute(); // Something strange happened
}
----------------
Please make sure we have a testcase that hits this "getCouldNotCompute".
================
Comment at: test/Analysis/ScalarEvolution/solve-quadratic-overflow.ll:2
+; RUN: opt -analyze -scalar-evolution -S < %s | FileCheck %s
+
+; CHECK: Loop %b1: backedge-taken count is 255
----------------
Please integrate this into the other testcase, with the same comments/CHECK lines.
================
Comment at: test/Analysis/ScalarEvolution/solve-quadratic.ll:1
+; RUN: opt -analyze -scalar-evolution -S -debug-only=scalar-evolution < %s 2>&1 | FileCheck %s
+
----------------
debug-only needs "REQUIRES: asserts".
Repository:
rL LLVM
https://reviews.llvm.org/D48283
More information about the llvm-commits
mailing list