[PATCH] D49428: [LSV] Look through selects for consecutive addresses

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:140
 
+  enum { RecursionLimit = 3 };
+
----------------
rtereshin wrote:
> arsenm wrote:
> > Why is this an enum?
> Sure, will change that to `static const` or `static constexpr`. Which one would you prefer?
Going with `static const` for now.


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:315
+bool Vectorizer::areConsecutivePointers(Value *PtrA, Value *PtrB,
+                                        APInt PtrDelta, unsigned MaxRecurse) {
   unsigned PtrBitWidth = DL.getPointerTypeSizeInBits(PtrA->getType());
----------------
rtereshin wrote:
> arsenm wrote:
> > Const method?
> Maybe, I never checked. Most likely not, it calls methods on `ScalarEvolution &SE`, which is part of the object, and those methods change the SCEVs' cache that is owned by `SE`, so most likely they aren't const methods themselves.
> 
> If it is a const method though, will do.
Could be done, good catch, doing that.


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:440-441
+
+  if (auto *SelectA = dyn_cast<SelectInst>(PtrA))
+    if (auto *SelectB = dyn_cast<SelectInst>(PtrB))
+      return SelectA->getCondition() == SelectB->getCondition() &&
----------------
rtereshin wrote:
> arsenm wrote:
> > Braces
> Where exactly would you like braces? First level if, second level if, or both?
> 
> I believe no curly braces around 1 statement bodies of if/then/else/for/while/do-while statements is the standard LLVM code style.
Going with braces around second level if for now.


Repository:
  rL LLVM

https://reviews.llvm.org/D49428