[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:323
+                                        APInt PtrDelta,
+                                        unsigned MaxRecurse) const {
   unsigned PtrBitWidth = DL.getPointerTypeSizeInBits(PtrA->getType());
----------------
volkan wrote:
> Replace `MaxRecurse` with `Depth` maybe?
ok


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:360
   // things the hard way.
-  return lookThroughComplexAddresses(PtrA, PtrB, BaseDelta);
+  return lookThroughComplexAddresses(PtrA, PtrB, BaseDelta, MaxRecurse);
 }
----------------
volkan wrote:
> Missing `--MaxRecurse`?
no, lookThroughComplexAddresses does not make recursive calls directly, lookThroughSelects does later.


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:368
   auto *GEPB = dyn_cast<GetElementPtrInst>(PtrB);
   if (!GEPA || !GEPB)
+    return lookThroughSelects(PtrA, PtrB, PtrDelta, MaxRecurse);
----------------
volkan wrote:
> You can check if  `isa<SelectInst>(A) && isa<SelectInst>(B)` here.
I can, but it gets messier.


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:454
 
+bool Vectorizer::lookThroughSelects(Value *PtrA, Value *PtrB, APInt PtrDelta,
+                                    unsigned MaxRecurse) const {
----------------
volkan wrote:
> You can replace `Value *` with `SelectInst &` and remove the dyn_casts below.
I can, but it gets messier on the caller's side.


Repository:
  rL LLVM

https://reviews.llvm.org/D49428