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

[Bruno Cardoso Lopes via llvm-commits]

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.


-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc