[PATCH] D36960: [X86][LLVM]Expanding Supports lowerInterleavedLoad() in X86InterleavedAccess (VF{8|16|32} stride 3).

[michael zuckerman via Phabricator via llvm-commits]

m_zuckerman marked 15 inline comments as done.
m_zuckerman added inline comments.


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:368
+
+  for (Index = Mask.size() - 1; Index >= 0; Index--) {
+    if (GroupNum != Mask[Index] % Stride) {
----------------
zvi wrote:
> I might be missing something, but is group number 0 defined to be the group starting at the end of Mask?
Group number zero ( According to the modulo operation) will always lie at the first element of the shuffle since we begin with zero.For example, think about a vector length 8 with stride 3. The shuffle sequence will look like that [0,3,6,1,4,7,2,5] so we don't know the last group but we can know what is the first element since we begin the shuffle from zero. 


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:441
+
+  createAlignMask(VecElems, Offset[0] + Offset[1], VPAlign2, VectorWide, true);
+  createAlignMask(VecElems, Offset[1], VPAlign3, VectorWide, true);
----------------
zvi wrote:
> So the last argument is always passed with the value true to all calls of this function?
No, the default mode is false like row 438. 


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:478
+
+  Value *TempVec = Builder.CreateShuffleVector(Vec[1], Vec[1], VPAlign3);
+  TransposedMatrix[0] = Builder.CreateShuffleVector(Vec[0], Vec[0], VPAlign2);
----------------
zvi wrote:
> The second vector argument should be an undef, and the shuffle mask should select only elements from the first vector operand.  This will eventually happen in InstCombine or DAGCombine, but why not avoid this compile-time overhead?
I want to reuse the createAlignMask function. The function according to two vectors and not one vector and one undef.


https://reviews.llvm.org/D36960