[PATCH] D25287: [SCEVAffinator] Make precise modular math more correct.

[Johannes Doerfert via llvm-commits]

jdoerfert added inline comments.


================
Comment at: lib/Support/SCEVAffinator.cpp:41
+// precisely.
+static unsigned const MaxSmallBitWidth = 7;
 
----------------
Meinersbur wrote:
> How is this magic number determined? Why is it constant and does not depend on the the type's width?
This is by definition a magic number we use to compare against the types bit-width. It was there before and it stems from some observations about the way ScalarEvolution handles low bit-width modulo expressions but that is basically it.


================
Comment at: lib/Support/SCEVAffinator.cpp:250
+      PWAC.first = addModuloSemantic(PWAC.first, Expr->getType());
     PWAC = checkForWrapping(Expr, PWAC);
   }
----------------
Meinersbur wrote:
> efriedma wrote:
> > jdoerfert wrote:
> > > Did you omit the `else` here on purpose? If we add modulo semantics there is no wrapping (by construction)
> > I can.  checkForWrapping is essentially a no-op on the result of addModuloSemantic, since the value can't actually be out of range.
> I don't understand why when the type's bitwidth is below a threshold, we always add modulo ie. ignore the nsw flag?
Right, we should not. addModuloSemantics should now probably the only place we should check for the nsw flag and exit if it is present.


Repository:
  rL LLVM

https://reviews.llvm.org/D25287