[PATCH] D93229: [VectorCombine] allow peeking through GEPs when creating a vector load

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Memory is confusing, pointing out some more potential issues.



================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:155-157
+    // The offset must be within a vector-length to allow shuffling into place.
+    if (Offset.uge(MinVectorSize / 8))
+      return false;
----------------
Is there a test coverage for edge case[s]?
Doesn't this check only ensure that we can load a single byte, not the entire element?


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:159
+
+    OffsetInBits = Offset.getZExtValue() * 8;
+    Alignment = commonAlignment(Alignment, Offset.getZExtValue());
----------------
Don't we need to ensure that the byte offset is a multiple of element size?


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:197
   SmallVector<int, 16> Mask(OutputNumElts, UndefMaskElem);
-  Mask[0] = 0;
+  Mask[0] = OffsetInBits / ScalarSize;
   VecLd = Builder.CreateShuffleVector(VecLd, Mask);
----------------
Assert that the division is exact?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93229/new/

https://reviews.llvm.org/D93229