[PATCH] D39286: [SCEV][NFC] Introduce isDivisorOf function in SCEV

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 26 00:04:22 PDT 2017


mkazantsev planned changes to this revision.
mkazantsev added a comment.

Going to add some stuff to make clear distinction between signed and unsigned division.



================
Comment at: lib/Analysis/ScalarEvolution.cpp:8963
 
+bool ScalarEvolution::isDivisorOf(const SCEV *S, const SCEV *Divisor) {
+  assert(getEffectiveSCEVType(S->getType()) ==
----------------
sanjoy wrote:
> This is really `isUnsignedDivisorOf`.
Not really. We can operate with negative values, for example of `S = 0` or `S = Divisor` or `Divisor = 1`. I also think that it will be OK to return true for `Divisor = -1` and `S = -Divisor`, and this is not be true in unsigned division. I am going to add these cases.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:8973
+  // and by one.
+  if (S == Divisor || S->isZero() || Divisor->isOne())
+    return true;
----------------
sanjoy wrote:
> If this is needed, you should add it to `getURemExpr`.
This is only needed to work with negative values in some particular cases. `getURemExpr` interprets all as positive. I want `isDivisorOf(-5, 5)` be true, but `(UINT_MAX - 4) urem 5 = 1`.



https://reviews.llvm.org/D39286





More information about the llvm-commits mailing list