[PATCH] D19661: [X86] Also try to zero elts when lowering v32i8 shuffle with PSHUFB.

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:11427-11436
@@ -11426,7 +11426,12 @@
 
-  if (isSingleInputShuffleMask(Mask)) {
-    // There are no generalized cross-lane shuffle operations available on i8
-    // element types.
-    if (is128BitLaneCrossingShuffleMask(MVT::v32i8, Mask))
-      return lowerVectorShuffleAsLanePermuteAndBlend(DL, MVT::v32i8, V1, V2,
-                                                     Mask, DAG);
+  SmallBitVector Zeroable = computeZeroableShuffleElements(Mask, V1, V2);
+  bool SingleInputMask = true;
+  bool SingleInputAndZeroesMask = true;
+  for (int i = 0, Size = Mask.size(); i < Size; ++i) {
+    if (Mask[i] >= Size) {
+      SingleInputMask = false;
+      if (!Zeroable[i])
+        SingleInputAndZeroesMask = false;
+    }
+  }
+  // There are no generalized cross-lane shuffle operations available on i8
----------------
Would it make sense to enhance isSingleInputShuffleMask() like:
bool isSingleInputShuffleMask(ArrayRef<int> Mask, bool OrZeroes = false) {}
and then optionally check for zeroes there?

It seems like vperm2* and maybe some other instructions would then be able to use the same helper.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:11445
@@ -11434,7 +11444,3 @@
     SDValue PSHUFBMask[32];
-    for (int i = 0; i < 32; ++i)
-      PSHUFBMask[i] =
-          Mask[i] < 0
-              ? DAG.getUNDEF(MVT::i8)
-              : DAG.getConstant(Mask[i] < 16 ? Mask[i] : Mask[i] - 16, DL,
-                                MVT::i8);
+    for (int i = 0; i < 32; ++i) {
+      if (Mask[i] < 0)
----------------
Please add a comment here to describe the pshufb encoding. Something like:
If the most-significant-bit of any byte of the PSHUFB mask is set, the destination byte is zeroed.
I hate having to look back at the manual. :)


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:11446
@@ +11445,3 @@
+    for (int i = 0; i < 32; ++i) {
+      if (Mask[i] < 0)
+        PSHUFBMask[i] = DAG.getUNDEF(MVT::i8);
----------------
Use SM_SentinelUndef?


http://reviews.llvm.org/D19661