SSE pslldq/psrldq shuffle mask decodes

[RKSimon]

Summary

Patch to provide shuffle decodes and asm comments for the sse pslldq/psrldq byte shift instructions.

Added comments checks to a number of vector insert/shuffle tests (we don't have that many uses cases of slldq/srldq at the moment).

[chandlerc]

lib/Target/X86/Utils/X86ShuffleDecode.cpp

83–84

I wouldn't decode this in terms of the vector element type. The mask should be a byte-vector mask in all cases, and VT should just provide the size of the vector (128-bit, 256-bit, 512-bit, whatever).

[RKSimon]

lib/Target/X86/Utils/X86ShuffleDecode.cpp

83–84

Yes I'd be happy to do that - I just followed the pattern from palignr. This does mean that the computed ShuffleMask count might not match the number of elements in the MVT. Would it be better to change the MVT VT argument to unsigned VectorSizeInBytes to make that clear?

[RKSimon]

The decoder always creates a byte-vector shuffle mask. I've altered the arguments to (hopefully) make this clear.

Rebased tests.

[RKSimon]

Ping.

Changes to the shuffle tests in the latest rebase have resulted in the psrldq test cases disappearing. This will be fixed with the followup patch to this that improves byte shift/rotate shuffles on pre-SSSE3 targets.

[qcolombet]

Hi Simon,

Could you add test cases with the ymm versions?

Those are the interesting ones :).

Thanks,
-Quentin

[RKSimon]

Hi Quentin,

Could you add test cases with the ymm versions?

Chandler can confirm but I don't think we have anything that creates vec256 byte rotates/shifts yet (and http://reviews.llvm.org/D5699 doesn't help either - it only improves the vec128 case) - if you wish I can remove the ymm support from X86InstComments.cpp to make this clear and add it back in a future patch that adds better support?

Simon.

[RKSimon]

Updated test cases for SSE2/AVX1/AVX2 intrinsics builtins to include the pslldq/psrldq shuffle mask decode in the comments

[chandlerc]

lib/Target/X86/InstPrinter/X86InstComments.cpp

207

This code doesn't really make sense. If you know the exact type you know the exact size, just pass 128.

But what I meant with my comment was still to pass the MVT down, but to do the getSizeInBits query inside the decode routine.

lib/Target/X86/Utils/X86ShuffleDecode.cpp

87

No need for braces aroun the outer loop.

The above loop also uses int and < which is my personal preference for these loops.

test/CodeGen/X86/vector-shuffle-128-v8.ll

1403

No need to give the operands here when checking the comment. It looks like all of the new comments in this file need the same treatment.

[RKSimon]

Chandler's recommendations: re-added vector type to decode function arguments, removed excess braces, for-loop comparison tidy up; removed operands in test cases with comments

[chandlerc]

test/CodeGen/X86/avx-intrinsics-x86.ll

461 ↗

(On Diff #14801)

This doesn't look right... this is the identity mask?

461 ↗

(On Diff #14801)

oh, this is bit shifting? what does this even mean? I'm not really sure how to interpret this test.

[RKSimon]

test/CodeGen/X86/avx-intrinsics-x86.ll

461 ↗

(On Diff #14801)

Yes its a bit shifting representation (you'll notice in D5699 I have to mutliply byte shifts by 8) - the lower 3 bits of the shift are lost (and ignored). The test was there already - I just added the decode comment - I agree it doesn't look that useful at first glance but it has use as an edge test and it does demonstrate that the decode works for zero shifts....

[chandlerc]

test/CodeGen/X86/avx-intrinsics-x86.ll

461 ↗

(On Diff #14801)

Wow. What a broken intrinsic. Anyways, sure.

[chandlerc]

And to be more explicit, LGTM, please commit. =]

[RKSimon]

Closed by commit rL219738 (authored by @RKSimon).

lib/Target/X86/InstPrinter/X86InstComments.cpp

     DecodeMOVSHDUPMask(MVT::v4f32, ShuffleMask);
     break;
 
+  case X86::PSLLDQri:
+  case X86::VPSLLDQri:
+    Src1Name = getRegName(MI->getOperand(1).getReg());
+    DestName = getRegName(MI->getOperand(0).getReg());
+    if(MI->getOperand(MI->getNumOperands()-1).isImm())
+      DecodePSLLDQMask(MVT(MVT::v16i8).getSizeInBits(),
+                       MI->getOperand(MI->getNumOperands()-1).getImm(),
+                       ShuffleMask);
+    break;
+
+  case X86::VPSLLDQYri:
+    Src1Name = getRegName(MI->getOperand(1).getReg());
+    DestName = getRegName(MI->getOperand(0).getReg());
+    if(MI->getOperand(MI->getNumOperands()-1).isImm())
+      DecodePSLLDQMask(MVT(MVT::v32i8).getSizeInBits(),
+                       MI->getOperand(MI->getNumOperands()-1).getImm(),
+                       ShuffleMask);
+    break;
+
+  case X86::PSRLDQri:
+  case X86::VPSRLDQri:
+    Src1Name = getRegName(MI->getOperand(1).getReg());
+    DestName = getRegName(MI->getOperand(0).getReg());
+    if(MI->getOperand(MI->getNumOperands()-1).isImm())
+      DecodePSRLDQMask(MVT(MVT::v16i8).getSizeInBits(),
+                       MI->getOperand(MI->getNumOperands()-1).getImm(),
+                       ShuffleMask);
+    break;
+
+  case X86::VPSRLDQYri:
+    Src1Name = getRegName(MI->getOperand(1).getReg());
+    DestName = getRegName(MI->getOperand(0).getReg());
+    if(MI->getOperand(MI->getNumOperands()-1).isImm())
+      DecodePSRLDQMask(MVT(MVT::v32i8).getSizeInBits(),
+                       MI->getOperand(MI->getNumOperands()-1).getImm(),
+                       ShuffleMask);
+    break;
+
   case X86::PALIGNR128rr:
   case X86::VPALIGNR128rr:
     Src1Name = getRegName(MI->getOperand(2).getReg());

lib/Target/X86/Utils/X86ShuffleDecode.h

 
 void DecodeMOVSHDUPMask(MVT VT, SmallVectorImpl<int> &ShuffleMask);
 
+void DecodePSLLDQMask(unsigned VectorSizeInBits, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
+
+void DecodePSRLDQMask(unsigned VectorSizeInBits, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
+
 void DecodePALIGNRMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
 
 void DecodePSHUFMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);

lib/Target/X86/Utils/X86ShuffleDecode.cpp

   }
 }
 
+void DecodePSLLDQMask(unsigned VectorSizeInBits, unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {
+  unsigned NumElts = VectorSizeInBits / 8;
+  unsigned NumLanes = VectorSizeInBits / 128;
+  unsigned NumLaneElts = NumElts / NumLanes;
+
+  for (unsigned l = 0; l != NumElts; l += NumLaneElts) {
+    for (unsigned i = 0; i != NumLaneElts; ++i) {
+      int M = SM_SentinelZero;
+      if (i >= Imm) M = i - Imm + l;
+      ShuffleMask.push_back(M);
+    }
+  }
+}
+
+void DecodePSRLDQMask(unsigned VectorSizeInBits, unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {
+  unsigned NumElts = VectorSizeInBits / 8;
+  unsigned NumLanes = VectorSizeInBits / 128;
+  unsigned NumLaneElts = NumElts / NumLanes;
+
+  for (unsigned l = 0; l != NumElts; l += NumLaneElts) {
+    for (unsigned i = 0; i != NumLaneElts; ++i) {
+      unsigned Base = i + Imm;
+      int M = Base + l;
+      if (Base >= NumLaneElts) M = SM_SentinelZero;
+      ShuffleMask.push_back(M);
+    }
+  }
+}
+
 void DecodePALIGNRMask(MVT VT, unsigned Imm,
                        SmallVectorImpl<int> &ShuffleMask) {
   unsigned NumElts = VT.getVectorNumElements();

test/CodeGen/X86/vector-shuffle-128-v8.ll

 ; SSE:       # BB#0:
 ; SSE-NEXT:    movzwl %di, %eax
 ; SSE-NEXT:    movd %eax, %xmm0
-; SSE-NEXT:    pslldq $2, %xmm0
+; SSE-NEXT:    pslldq $2, %xmm0 {{.*#+}} xmm0 = zero,zero,xmm0[0,1,2,3,4,5,6,7,8,9,10,11,12,13]
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: shuffle_v8i16_z8zzzzzz:

 ; AVX:       # BB#0:
 ; AVX-NEXT:    movzwl %di, %eax
 ; AVX-NEXT:    vmovd %eax, %xmm0
-; AVX-NEXT:    vpslldq $2, %xmm0, %xmm0
+; AVX-NEXT:    vpslldq $2, %xmm0, %xmm0 {{.*#+}} xmm0 = zero,zero,xmm0[0,1,2,3,4,5,6,7,8,9,10,11,12,13]
 ; AVX-NEXT:    retq
   %a = insertelement <8 x i16> undef, i16 %i, i32 0
   %shuffle = shufflevector <8 x i16> zeroinitializer, <8 x i16> %a, <8 x i32> <i32 2, i32 8, i32 3, i32 7, i32 6, i32 5, i32 4, i32 3>

 ; SSE:       # BB#0:
 ; SSE-NEXT:    movzwl %di, %eax
 ; SSE-NEXT:    movd %eax, %xmm0
-; SSE-NEXT:    pslldq $10, %xmm0
+; SSE-NEXT:    pslldq $10, %xmm0 {{.*#+}} xmm0 = zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,xmm0[0,1,2,3,4,5]
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: shuffle_v8i16_zzzzz8zz:

 ; AVX:       # BB#0:
 ; AVX-NEXT:    movzwl %di, %eax
 ; AVX-NEXT:    vmovd %eax, %xmm0
-; AVX-NEXT:    vpslldq $10, %xmm0, %xmm0
+; AVX-NEXT:    vpslldq $10, %xmm0, %xmm0 {{.*#+}} xmm0 = zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,xmm0[0,1,2,3,4,5]
 ; AVX-NEXT:    retq
   %a = insertelement <8 x i16> undef, i16 %i, i32 0
   %shuffle = shufflevector <8 x i16> zeroinitializer, <8 x i16> %a, <8 x i32> <i32 0, i32 0, i32 0, i32 0, i32 0, i32 8, i32 0, i32 0>

 ; SSE:       # BB#0:
 ; SSE-NEXT:    movzwl %di, %eax
 ; SSE-NEXT:    movd %eax, %xmm0
-; SSE-NEXT:    pslldq $14, %xmm0
+; SSE-NEXT:    pslldq $14, %xmm0 {{.*#+}} xmm0 = zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,xmm0[0,1]
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: shuffle_v8i16_zuuzuuz8:

 ; AVX:       # BB#0:
 ; AVX-NEXT:    movzwl %di, %eax
 ; AVX-NEXT:    vmovd %eax, %xmm0
-; AVX-NEXT:    vpslldq $14, %xmm0, %xmm0
+; AVX-NEXT:    vpslldq $14, %xmm0, %xmm0 {{.*#+}} xmm0 = zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,xmm0[0,1]
 ; AVX-NEXT:    retq
   %a = insertelement <8 x i16> undef, i16 %i, i32 0
   %shuffle = shufflevector <8 x i16> zeroinitializer, <8 x i16> %a, <8 x i32> <i32 0, i32 undef, i32 undef, i32 3, i32 undef, i32 undef, i32 6, i32 8>

 ; SSE:       # BB#0:
 ; SSE-NEXT:    movzwl %di, %eax
 ; SSE-NEXT:    movd %eax, %xmm0
-; SSE-NEXT:    pslldq $4, %xmm0
+; SSE-NEXT:    pslldq $4, %xmm0 {{.*#+}} xmm0 = zero,zero,zero,zero,xmm0[0,1,2,3,4,5,6,7,8,9,10,11]
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: shuffle_v8i16_zzBzzzzz:

 ; AVX:       # BB#0:
 ; AVX-NEXT:    movzwl %di, %eax
 ; AVX-NEXT:    vmovd %eax, %xmm0
-; AVX-NEXT:    vpslldq $4, %xmm0, %xmm0
+; AVX-NEXT:    vpslldq $4, %xmm0, %xmm0 {{.*#+}} xmm0 = zero,zero,zero,zero,xmm0[0,1,2,3,4,5,6,7,8,9,10,11]
 ; AVX-NEXT:    retq
   %a = insertelement <8 x i16> undef, i16 %i, i32 3
   %shuffle = shufflevector <8 x i16> zeroinitializer, <8 x i16> %a, <8 x i32> <i32 0, i32 1, i32 11, i32 3, i32 4, i32 5, i32 6, i32 7>