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

Zvi Rackover via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 12 05:06:44 PDT 2017


zvi added inline comments.


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:218
+//  We are assuming that VF*type = half Xmm.
+static void createLowUnpackMask(int NumElements,
+                                SmallVectorImpl<uint32_t> &Mask, MVT VT) {
----------------
'unsigned' looks like a more suitable type, That will also allow 'i' in the for loop below to be of 'unsigned' type, and that is better because it is being pushed into Mask, a container of type 'uint32_t'.


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:228
+
+// Changing the scale of the vector type by reducing the number of elements and
+// doubling the scalar size.
----------------
The comment doesn't address the special case v8i8. Can you please explain why the exception is needed?


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:230
+// doubling the scalar size.
+static MVT scaleVevtorType(MVT VT) {
+  if (VT == MVT::v8i8)
----------------
*scaleVectorType


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:248
 
-  Type *VecTyepVt = VectorType::get(Type::getInt8Ty(Shuffles[0]->getContext()),
-                                    numberOfElement);
-  Type *VecTyepVtHalf = VectorType::get(
-      Type::getInt16Ty(Shuffles[0]->getContext()), numberOfElement / 2);
-  MVT VT = MVT::getVT(VecTyepVt);
-  MVT HalfVT = MVT::getVT(VecTyepVtHalf);
+  MVT VT = MVT::getVectorVT(MVT::i8, numberOfElement);
+  MVT HalfVT = scaleVevtorType(VT);
----------------
Can you please commit these minor changes: leaner computation of VT and Half, variable renaming in a couple of lines below as an NFC commit and rebase this patch?


================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:279
+  Value *IntrVec1Low, *IntrVec1High, *IntrVec2Low, *IntrVec2High;
+  if (VT == MVT::v8i8) {
+    TransposedMatrix.resize(2);
----------------
I think it would be better to create a function that handles the v8i8 case instead of having these if's + early exit. It will also allow reomving the  if from scaleVectorType to the point where it may not be needed.


https://reviews.llvm.org/D36058





More information about the llvm-commits mailing list