[llvm] r251207 - [X86][SSE] lowerVectorShuffleWithUNPCK - use equivalent shuffle mask test.

[Simon Pilgrim via llvm-commits]

> On 26 Oct 2015, at 19:02, Bruno Cardoso Lopes <bruno.cardoso at gmail.com> wrote:
> 
> Hi Simon,
> 
> Very nice.
> 
> On Sat, Oct 24, 2015 at 1:48 PM, Simon Pilgrim via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: rksimon
>> Date: Sat Oct 24 15:48:08 2015
>> New Revision: 251207
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=251207&view=rev
>> Log:
>> [X86][SSE] lowerVectorShuffleWithUNPCK - use equivalent shuffle mask test.
>> 
>> Use isShuffleEquivalent to match UNPCK shuffles - better support for build vector inputs.
>> 
>> Modified:
>>    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=251207&r1=251206&r2=251207&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat Oct 24 15:48:08 2015
>> @@ -6785,43 +6785,32 @@ static SDValue lowerVectorShuffleWithUNP
>>                                            SDValue V1, SDValue V2,
>>                                            SelectionDAG &DAG) {
>>   int NumElts = VT.getVectorNumElements();
>> -  bool Unpckl = true;
>> -  bool Unpckh = true;
>> -  bool UnpcklSwapped = true;
>> -  bool UnpckhSwapped = true;
>>   int NumEltsInLane = 128 / VT.getScalarSizeInBits();
>> +  SmallVector<int, 8> Unpckl;
>> +  SmallVector<int, 8> Unpckh;
>> 
>>   for (int i = 0; i < NumElts; ++i) {
>>     unsigned LaneStart = (i / NumEltsInLane) * NumEltsInLane;
>> -
>>     int LoPos = (i % NumEltsInLane) / 2 + LaneStart + NumElts * (i % 2);
>>     int HiPos = LoPos + NumEltsInLane / 2;
>> -    int LoPosSwapped = (LoPos + NumElts) % (NumElts * 2);
>> -    int HiPosSwapped = (HiPos + NumElts) % (NumElts * 2);
>> -
>> -    if (Mask[i] == -1)
>> -      continue;
>> -    if (Mask[i] != LoPos)
>> -      Unpckl = false;
>> -    if (Mask[i] != HiPos)
>> -      Unpckh = false;
>> -    if (Mask[i] != LoPosSwapped)
>> -      UnpcklSwapped = false;
>> -    if (Mask[i] != HiPosSwapped)
>> -      UnpckhSwapped = false;
>> -    if (!Unpckl && !Unpckh && !UnpcklSwapped && !UnpckhSwapped)
>> -      return SDValue();
>> +    Unpckl.push_back(LoPos);
>> +    Unpckh.push_back(HiPos);
>>   }
>> -  if (Unpckl)
>> +
>> +  if (isShuffleEquivalent(V1, V2, Mask, Unpckl))
>>     return DAG.getNode(X86ISD::UNPCKL, DL, VT, V1, V2);
>> -  if (Unpckh)
>> +  if (isShuffleEquivalent(V1, V2, Mask, Unpckh))
>>     return DAG.getNode(X86ISD::UNPCKH, DL, VT, V1, V2);
>> -  if (UnpcklSwapped)
>> +
>> +  // Commute and try again.
>> +  ShuffleVectorSDNode::commuteMask(Unpckl);
>> +  if (isShuffleEquivalent(V1, V2, Mask, Unpckl))
>>     return DAG.getNode(X86ISD::UNPCKL, DL, VT, V2, V1);
>> -  if (UnpckhSwapped)
>> +
>> +  ShuffleVectorSDNode::commuteMask(Unpckh);
>> +  if (isShuffleEquivalent(V1, V2, Mask, Unpckh))
>>     return DAG.getNode(X86ISD::UNPCKH, DL, VT, V2, V1);
>> 
>> -  llvm_unreachable("Unexpected result of UNPCK mask analysis");
> 
> Did you hit unreachable while working on this? I agree that it's
> better to fallback than break the compiler, but if there's a known
> path here it deserves a test case.

The old code had the unreachable as it had an early out if all the patterns failed, this patch removes it. As we’re now calling isShuffleEquivalent to test each pattern separately, there isn't an early out and if all the tests fail the code just flows to the end, returns SDValue() and continues lowering in the calling function.