[PATCH] D13252: [InstCombine] Teach how to convert SSSE3/AVX2 byte shuffles to builtin shuffles if the shuffle mask is constant.

[Andrea Di Biagio via llvm-commits]

andreadb added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1197
@@ +1196,3 @@
+    for (unsigned I = 16; I < NumElts; ++I)
+      Indexes[I] += 16;
+
----------------
RKSimon wrote:
> I'm worried this is hardcoded to the AVX2 '32 element' implementation (so if the AVX512 intrinsic ever gets supported here...)
> 
> You could possibly merge this into the code above:
> 
> Indexes[I] = (I & ~0xF) + ((Index < 0) ? NumElts : (Index & 0xF))
Yes, this is done specifically to support the AVX2 implementation.
However, I don't think we can merge that into the code above because we also have to handle ConstantAggregateZero pshufb masks.

================
Comment at: test/Transforms/InstCombine/x86-pshufb.ll:2
@@ +1,3 @@
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+; Verify that instcombine is able to fold identity shuffles.
----------------
RKSimon wrote:
> You have a decent range of tests here already - the only addition I can think of would be to show that some 'strange value' pshufb indices (i.e. not -128 or 0-15 or 0-31) get correctly masked based on their bits.
Sure, I will add extra tests for it.


http://reviews.llvm.org/D13252