[llvm-commits] [llvm] r127630 - in /llvm/trunk: lib/Target/ARM/ARMISelDAGToDAG.cpp lib/Target/ARM/ARMISelLowering.cpp lib/Target/ARM/ARMISelLowering.h test/CodeGen/ARM/vext.ll

[Bob Wilson]

On Mar 14, 2011, at 4:02 PM, Bill Wendling wrote:

> Author: void
> Date: Mon Mar 14 18:02:38 2011
> New Revision: 127630
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=127630&view=rev
> Log:
> Generate a VTBL instruction instead of a series of loads and stores when we
> can.

You need to update ARMTargetLowering::isShuffleMaskLegal to show that all v8i8 shuffles are legal.
Other comments below....

> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=127630&r1=127629&r2=127630&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Mon Mar 14 18:02:38 2011
> @@ -852,6 +852,10 @@
>   case ARMISD::VZIP:          return "ARMISD::VZIP";
>   case ARMISD::VUZP:          return "ARMISD::VUZP";
>   case ARMISD::VTRN:          return "ARMISD::VTRN";
> +  case ARMISD::VTBL1:         return "ARMISD::VTBL1";
> +  case ARMISD::VTBL2:         return "ARMISD::VTBL2";
> +  case ARMISD::VTBL3:         return "ARMISD::VTBL3";
> +  case ARMISD::VTBL4:         return "ARMISD::VTBL4";

Since you're only using VTBL1 and VTBL2, there's no need for the 3 & 4 versions.

>   case ARMISD::VMULLs:        return "ARMISD::VMULLs";
>   case ARMISD::VMULLu:        return "ARMISD::VMULLu";
>   case ARMISD::BUILD_VECTOR:  return "ARMISD::BUILD_VECTOR";
> @@ -4055,6 +4059,29 @@
>   }
> }
> 
> +static SDValue LowerVECTOR_SHUFFLEv8i8(SDValue Op,
> +                                       SmallVectorImpl<int> &ShuffleMask,
> +                                       SelectionDAG &DAG) {
> +  // Check to see if we can use the VTBL instruction.
> +  SDValue V1 = Op.getOperand(0);
> +  SDValue V2 = Op.getOperand(1);
> +  DebugLoc DL = Op.getDebugLoc();
> +
> +  SmallVector<SDValue, 8> VTBLMask;
> +  for (SmallVectorImpl<int>::iterator
> +         I = ShuffleMask.begin(), E = ShuffleMask.end(); I != E; ++I)
> +    VTBLMask.push_back(DAG.getConstant(*I, MVT::i32));
> +
> +  if (V2.getNode()->getOpcode() == ISD::UNDEF)
> +    return DAG.getNode(ARMISD::VTBL1, DL, MVT::v8i8, V1,
> +                       DAG.getNode(ISD::BUILD_VECTOR, DL, MVT::v8i8,
> +                                   &VTBLMask[0], 8));
> +  else

There's no need for that "else".

> +    return DAG.getNode(ARMISD::VTBL2, DL, MVT::v8i8, V1, V2, 
> +                       DAG.getNode(ISD::BUILD_VECTOR, DL, MVT::v8i8,
> +                                   &VTBLMask[0], 8));
> +}
> +
> static SDValue LowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG) {
>   SDValue V1 = Op.getOperand(0);
>   SDValue V2 = Op.getOperand(1);
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.h?rev=127630&r1=127629&r2=127630&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.h (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.h Mon Mar 14 18:02:38 2011
> @@ -153,6 +153,10 @@
>       VZIP,         // zip (interleave)
>       VUZP,         // unzip (deinterleave)
>       VTRN,         // transpose
> +      VTBL1,        // 1-register shuffle with mask
> +      VTBL2,        // 2-register shuffle with mask
> +      VTBL3,        // 3-register shuffle with mask
> +      VTBL4,        // 4-register shuffle with mask

See comment above about VTBL3 and VTBL4.

> 
>       // Vector multiply long:
>       VMULLs,       // ...signed
> 
> Modified: llvm/trunk/test/CodeGen/ARM/vext.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/vext.ll?rev=127630&r1=127629&r2=127630&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/vext.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/vext.ll Mon Mar 14 18:02:38 2011
> @@ -121,15 +121,3 @@
>         %tmp2 = shufflevector <8 x i16> %tmp1, <8 x i16> undef, <4 x i32> <i32 0, i32 2, i32 4, i32 6>
>         ret <4 x i16> %tmp2
> }
> -
> -; The actual shuffle code only handles some cases, make sure we check
> -; this rather than blindly emitting a VECTOR_SHUFFLE (infinite
> -; lowering loop can result otherwise).
> -define <8 x i8> @test_illegal(<16 x i8>* %A, <16 x i8>* %B) nounwind {
> -;CHECK: test_illegal:
> -;CHECK: vst1.8
> -       %tmp1 = load <16 x i8>* %A
> -       %tmp2 = load <16 x i8>* %B
> -       %tmp3 = shufflevector <16 x i8> %tmp1, <16 x i8> %tmp2, <8 x i32> <i32 0, i32 7, i32 5, i32 25, i32 3, i32 2, i32 2, i32 26>
> -       ret <8 x i8> %tmp3
> -}

Instead of removing this testcase, can you change it to test an illegal v8i16 shuffle?