[PATCH] D15412: [SCEV][LAA] Add no overflow SCEV predicates and use use them to improve strided pointer detection

[silviu.baranga@arm.com via llvm-commits]

sbaranga added inline comments.

================
Comment at: include/llvm/Analysis/ScalarEvolution.h:286
@@ +285,3 @@
+    ///
+    /// 0 <= F(a) + G(b) < 2^n
+    ///
----------------
sanjoy wrote:
> Do you mean `G(a) + F(b)` here?  Might be helpful to name these as `Signed` and `Unsigned` instead of `F` and `G`.
> 
> Might be useful to explicitly note that `SCEVWrapPredicate:: IncrementNUW` is **not** commutative, and you're interpreting `{a,+,b}` as `(((a + b) + b) + b) ...` for the purposes of `SCEVWrapPredicate`.
Good catch, it should have been G(a) + F(b). It is indeed not commutative. I'll update the text to reflect this.

================
Comment at: include/llvm/Analysis/ScalarEvolution.h:300
@@ +299,3 @@
+               SCEVWrapPredicate::IncrementWrapFlags OffFlags) {
+      assert(Flags <= IncrementNoWrapMask && Flags >= IncrementAnyWrap &&
+             "Invalid flags value!");
----------------
sanjoy wrote:
> What you have here is fine, but I'd have done `assert(Flags & IncrementNoWrapMask == Flags)` instead.
That seems a better way to do it, thanks.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:9741
@@ +9740,3 @@
+  if (ScalarEvolution::setFlags(ScevFlags, SCEV::FlagNSW) == ScevFlags)
+    IFlags = clearFlags(IFlags, IncrementNSW);
+
----------------
sanjoy wrote:
> Minor and optional, but why not have an early return here as `return IFlags == IncrementNSW;`?
This is a corner case, but that wouldn't work if Flags == IncrementAnyWrap (in which case the predicate would always be true).


http://reviews.llvm.org/D15412