[PATCH] D36058: [X86][LLVM]Expanding Supports lowerInterleavedStore() in X86InterleavedAccess (VF8 stride 4).

[Farhana Aleen via Phabricator via llvm-commits]

Farhana added inline comments.


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:124
+  if (ShuffleElemSize == 8 && isa<StoreInst>(Inst) &&
+      (WideInstSize == 256 || WideInstSize == 512 || WideInstSize == 1024))
+    return true;
----------------
Please remove the extra space after "&&". 



================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:196
+//  The shuffle pattern is as following :
+//  (0, NumElements, 1, NumElements + 1, …, NumElements - 1,
+//  2 * NumElements - 1).
----------------
Please remove the unknown character.


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:200
+
+static void createLowUnpackMask(int NumElements,
+                                SmallVectorImpl<uint32_t> &Mask,MVT VT) {
----------------
Is there anything special about this unpackMask? Why do we need a separate unpackmask generator? Why aren't you extending the createUnpackShuffleMask()? If createLowUnpackMask is unpacking in a different way than createUnpackShuffleMask() that distinction should be documented and clearly noted why we need a separate unpackMask-creator.  


https://reviews.llvm.org/D36058