[PATCH] D46191: [LV] Preserve inbounds on created GEPs

[Hideki Saito via Phabricator via llvm-commits]

hsaito added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3128
 
+    bool InBounds = false;
+    if (auto *gep = dyn_cast<GetElementPtrInst>(
----------------
Would you try using getLoadStorePointerOperand() from Instructions.h (see r327173) and hoist this code above "if (SI)", to cover both LI and SI at the same time?



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3160
+          PartPtr = cast<GetElementPtrInst>(
+              Builder.CreateGEP(nullptr, PartPtr, Builder.getInt32(1 - VF)));
+          PartPtr->setIsInBounds(InBounds);
----------------
I wonder why this isn't done using a single GEP...... If we use a single GEP, we can clean up a little more.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3197
       // Calculate the pointer for the specific unroll-part.
-      Value *PartPtr =
-          Builder.CreateGEP(nullptr, Ptr, Builder.getInt32(Part * VF));
+      auto *PartPtr = cast<GetElementPtrInst>(
+          Builder.CreateGEP(nullptr, Ptr, Builder.getInt32(Part * VF)));
----------------
Address computation part of LI and SI code paths should be identical. We probably should clean this up, but I suppose I shouldn't be asking you to do that as part of this patch. Just a note.


Repository:
  rL LLVM

https://reviews.llvm.org/D46191