[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:287
@@ +286,3 @@
+    /// \brief Returns the AddRec expression that we've made assumptions for.
+    const SCEV *getExpr() const;
+
----------------
sanjoy wrote:
> Nit: I'd return an `SCEVAddRecExpr *`.
This is part of the SCEVPredicate interface (and is missing an override), so it has to return a a const SCEV *.

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:2059
@@ +2058,3 @@
+  Value *OfAdd = OFBuilder.CreateExtractValue(Add, 1, "add.overflow");
+  Value *Overflow = OFBuilder.CreateOr(OfMul, OfAdd, "overflow");
+
----------------
sbaranga wrote:
> sanjoy wrote:
> > Actually, now that I think of it, I don't think this scheme will work for `nsw`.  E.g. if `AR` is `i8 {0,+,1}` and `TripCount` is `i8 255` == `i8 -1` then neither `(i8 1) * (i8 255)` == `i8 -1` nor `0 + (-1)` will sign overflow, while the add recurrence clearly does sign overflow.
> > 
> > I think the most obvious way to check for no `nsw` is to check `sext(Start + TripCount * Step)` == `sext(Start) + zext(TripCount) * sext(Step)`, where you extend to 2 * Bitwidth(Start).  There could also potentially be a more efficient scheme, but I don't have one with me right now.
> Thanks for the catch! This is indeed a problem. I'll think about some more alternatives to this.
We might be able to check this without extending by writing TripCount as MAX_INT + x if it larger than MAX_INT,
and doing the check twice? I don't know if this will be more efficient.

This also got me thinking about negative increments as well, and I think that there is an issue with the nuw flag and negative increments: they will always wrap.

We actually want a property that will allow us to sign extend the increment for both the nuw and nsw cases:

zext({a,+,b}) -> {zext(a),+,sext(b)}
sext({a,+,b}) -> {sext(a),+,sext(b)}

We should define separate flags for this property, as the unsigned case doesn't have the same semantics as the nuw flag. I think we should be able to emit checks for this.


http://reviews.llvm.org/D15412