[PATCH] D13595: [SCEV][LV] Add SCEV Predicates and use them to re-implement stride versioning

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

sbaranga marked an inline comment as done.
sbaranga added a comment.

Is this ok to commit? Maybe we could maybe handle the getFirstInst as a follow-up?

Thanks,
Silviu


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9317
@@ +9316,3 @@
+
+static Instruction *getFirstInst(Instruction *FirstInst, Value *V,
+                                 Instruction *Loc) {
----------------
sanjoy wrote:
> Can we name this function better?  I don't have a better suggestion though.
I don't have any good idea here.

It should also probably be moved into some place where it can be shared with LoopAccessAnalysis, but I don't know where exactly it would fit.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:9322
@@ +9321,3 @@
+  if (auto *I = dyn_cast<Instruction>(V))
+    return I->getParent() == Loc->getParent() ? I : nullptr;
+  return nullptr;
----------------
sanjoy wrote:
> When can `I->getParent()` be not equal to `Loc->getParent()`?
This was part of the original "getFirstInst" implementation lifted out of LoopVectorize. This can potentially happen when theIRBuilder is folding instructions outside of the current basic block produced by the SCEV expander (I think the SCEV expander is able to produce such instructions - at least for SCEVUnknowns).


http://reviews.llvm.org/D13595