[PATCH] D49428: [LSV] Look through selects for consecutive addresses
Roman Tereshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 19 11:56:11 PDT 2018
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
More information about the llvm-commits
mailing list