[PATCH] Fix assert when decoding PSHUFB mask

Robert Lougher rob.lougher at gmail.com
Fri Sep 19 10:46:51 PDT 2014


Hi,

Revision 213986 added code to decode PSHUFB masks in order to print
shuffle comments for PSHUFB instructions that have a constant mask.
This decode logic was then used in the new shuffle lowering code
(r214628).

The mask decode routine asserts if the mask index is out of range (<0
or greater than the vector size).  The problem is, we can legitimately
have a PSHUFB with a large index using the intrinsic:

define <16 x i8> @test(<16 x i8> %V) {
  %1 = tail call <16 x i8> @llvm.x86.ssse3.pshuf.b.128(<16 x i8> %V,
<16 x i8> <i8 0, i8 50, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8
0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>)
  ret <16 x i8> %1
}

Here we have an index of 50, which will assert.  The instruction,
however, takes the least significant 4 bits, so this is index 2.

The attached patch matches this behaviour, and removes the assert (the
index can never be negative).  The test simply checks that we don't
assert.

Please review and let me know if it is OK to commit.

Thanks,
Rob.

--

Robert Lougher
SN Systems - Sony Computer Entertainment Group
-------------- next part --------------
Index: lib/Target/X86/Utils/X86ShuffleDecode.cpp
===================================================================
--- lib/Target/X86/Utils/X86ShuffleDecode.cpp	(revision 218123)
+++ lib/Target/X86/Utils/X86ShuffleDecode.cpp	(working copy)
@@ -247,9 +247,8 @@
     if (Element & (1 << 7))
       ShuffleMask.push_back(SM_SentinelZero);
     else {
-      int Index = Base + Element;
-      assert((Index >= 0 && Index < NumElements) &&
-             "Out of bounds shuffle index for pshub instruction!");
+      // Only the least significant 4 bits of the byte are used.
+      int Index = Base + (Element & 0xf);
       ShuffleMask.push_back(Index);
     }
   }
@@ -266,9 +265,8 @@
     if (M & (1 << 7))
       ShuffleMask.push_back(SM_SentinelZero);
     else {
-      int Index = Base + M;
-      assert((Index >= 0 && (unsigned)Index < RawMask.size()) &&
-             "Out of bounds shuffle index for pshub instruction!");
+      // Only the least significant 4 bits of the byte are used.
+      int Index = Base + (M & 0xf);
       ShuffleMask.push_back(Index);
     }
   }
Index: test/CodeGen/X86/pshufb-intrinsic-crash.ll
===================================================================
--- test/CodeGen/X86/pshufb-intrinsic-crash.ll	(revision 0)
+++ test/CodeGen/X86/pshufb-intrinsic-crash.ll	(working copy)
@@ -0,0 +1,11 @@
+; RUN: llc < %s -march=x86-64 -mcpu=corei7
+; REQUIRES: asserts
+
+; Test that the compiler doesn't assert if we use a pshufb intrinsic with a mask index that
+; is larger than the size of the vector.
+
+define <16 x i8> @test(<16 x i8> %V) {
+  %1 = tail call <16 x i8> @llvm.x86.ssse3.pshuf.b.128(<16 x i8> %V, <16 x i8> <i8 0, i8 50, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>)
+  ret <16 x i8> %1
+}
+declare <16 x i8> @llvm.x86.ssse3.pshuf.b.128(<16 x i8>, <16 x i8>) nounwind readnone


More information about the llvm-commits mailing list