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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Ok, this looks about right to me.

I'm not sure if we'll want to lift the restrictions that

  (1) the offset must be a multiple of element size

and (2) that we try to load from the base pointer, which only works if the offset is small enough.

I've checked, and alive2 claims that this is endianness-agnostic.

Thanks.



================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:145
   auto *MinVecTy = VectorType::get(ScalarTy, MinVecNumElts, false);
-  if (!isSafeToLoadUnconditionally(SrcPtr, MinVecTy, Align(1), DL, Load, &DT))
-    return false;
+  unsigned OffsetInBits = 0;
+  Align Alignment = Load->getAlign();
----------------
And now that we already calculate `NumEltsInOffset`,
shall we just use/record `NumEltsInOffset` instead of recalculating it from `OffsetInBits` later?
(It them might use a better name, something like `ElementIndex` maybe)


================
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);
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > Assert that the division is exact?
> And also assert that `Mask[0] < MinVecNumElts`.
(If we no longer recompute them here, the asserts can obviously go away to)


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

https://reviews.llvm.org/D93229