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

[Volkan Keles via Phabricator via llvm-commits]

volkan added a comment.

Hi Roman,

I added a few comments. Could you take a look?



================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:147
 
+  static const unsigned RecursionLimit = 3;
+
----------------
Replace `RecursionLimit ` with `MaxDepth` maybe?


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:323
+                                        APInt PtrDelta,
+                                        unsigned MaxRecurse) const {
   unsigned PtrBitWidth = DL.getPointerTypeSizeInBits(PtrA->getType());
----------------
Replace `MaxRecurse` with `Depth` maybe?


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:360
   // things the hard way.
-  return lookThroughComplexAddresses(PtrA, PtrB, BaseDelta);
+  return lookThroughComplexAddresses(PtrA, PtrB, BaseDelta, MaxRecurse);
 }
----------------
Missing `--MaxRecurse`?


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


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:454
 
+bool Vectorizer::lookThroughSelects(Value *PtrA, Value *PtrB, APInt PtrDelta,
+                                    unsigned MaxRecurse) const {
----------------
You can replace `Value *` with `SelectInst &` and remove the dyn_casts below.


Repository:
  rL LLVM

https://reviews.llvm.org/D49428