[llvm] r311892 - [DAGCombiner] Teach visitEXTRACT_SUBVECTOR to turn extracts of BUILD_VECTOR into smaller BUILD_VECTORs

via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 31 01:40:42 PDT 2017


Here’s what it looks like.

I added debug statements as follows:

  fprintf(stderr,"foo\n");
  V.dump();
  // Skip bitcasting
  V = peekThroughBitcast(V);
  V.dump();

  // If the input is a build vector. Try to make a smaller build vector.
  if (V->getOpcode() == ISD::BUILD_VECTOR) {
    if (auto *Idx = dyn_cast<ConstantSDNode>(N->getOperand(1))) {
      EVT InVT = V->getValueType(0);
      unsigned NumElems = NVT.getSizeInBits() / InVT.getScalarSizeInBits();
      fprintf(stderr,"%d %d %d\n", NVT.getSizeInBits(), InVT.getScalarSizeInBits(), NumElems);
      if (NumElems > 0) {
        EVT ExtractVT = EVT::getVectorVT(*DAG.getContext(),
                                         InVT.getVectorElementType(), NumElems);
        if (!LegalOperations ||
            TLI.isOperationLegal(ISD::BUILD_VECTOR, ExtractVT)) {
          unsigned IdxVal = Idx->getZExtValue() * NVT.getScalarSizeInBits() /
                            InVT.getScalarSizeInBits();
          fprintf(stderr,"%d\n", IdxVal);

          // Extract the pieces from the original build_vector.
          SDValue BuildVec = DAG.getBuildVector(ExtractVT, SDLoc(N),
                                            makeArrayRef(V->op_begin() + IdxVal,
                                                         NumElems));
          BuildVec.dump();
          return DAG.getBitcast(NVT, BuildVec);
        }
      }
    }
  }

This results in the following printout

Combining: t12: v3f16 = extract_subvector t168, Constant:i32<0>
foo
t168: v4f16 = bitcast t167
t167: v2f32 = BUILD_VECTOR t164, t166
48 32 1
0
Creating new node: t170: v1f32 = BUILD_VECTOR t164
t170: v1f32 = BUILD_VECTOR t164
Assertion failed: (VT.getSizeInBits() == Operand.getValueSizeInBits() && "Cannot BITCAST between types of different sizes!"), function getNode, file ../lib/CodeGen/SelectionDAG/SelectionDAG.cpp, line 37
97.

Note how what it’s attempting to do here just… doesn’t seem to make much sense, per the printout.

—escha

> On Aug 30, 2017, at 3:34 PM, Topper, Craig <craig.topper at intel.com> wrote:
> 
> Hi Escha,
>  
> This should be calculating the type for the BUILD_VECTOR with same scalar size but less elements. It uses NVT to get the result vector width, and InVT to get the scalar size of the original BUILD_VECTOR. If InVT’s element size is bigger than NVT it gives up.
>  
>       unsigned NumElems = NVT.getSizeInBits() / InVT.getScalarSizeInBits();
>       if (NumElems > 0) {
>         EVT ExtractVT = EVT::getVectorVT(*DAG.getContext(),
>                                          InVT.getVectorElementType(), NumElems);
>  
> Do you have any additional information?
>   <>
>  <>From: fglaser at apple.com [mailto:fglaser at apple.com] On Behalf Of escha at apple.com
> Sent: Wednesday, August 30, 2017 3:05 PM
> To: Topper, Craig <craig.topper at intel.com>
> Cc: llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r311892 - [DAGCombiner] Teach visitEXTRACT_SUBVECTOR to turn extracts of BUILD_VECTOR into smaller BUILD_VECTORs
>  
> This breaks a test on our backend, but I suspect the patch is slightly bugged. Note the following:
>  
>   // Skip bitcasting
>   V = peekThroughBitcast(V);
>  
>   // If the input is a build vector. Try to make a smaller build vector.
>   if (V->getOpcode() == ISD::BUILD_VECTOR) {
>     if (auto *Idx = dyn_cast<ConstantSDNode>(N->getOperand(1))) {
>       EVT InVT = V->getValueType(0);
>       unsigned NumElems = NVT.getSizeInBits() / InVT.getScalarSizeInBits();
>       if (NumElems > 0) {
>         EVT ExtractVT = EVT::getVectorVT(*DAG.getContext(),
>                                          InVT.getVectorElementType(), NumElems);
>         if (!LegalOperations ||
>             TLI.isOperationLegal(ISD::BUILD_VECTOR, ExtractVT)) {
>           unsigned IdxVal = Idx->getZExtValue() * NVT.getScalarSizeInBits() /
>                             InVT.getScalarSizeInBits();
>  
>           // Extract the pieces from the original build_vector.
>           SDValue BuildVec = DAG.getBuildVector(ExtractVT, SDLoc(N),
>                                             makeArrayRef(V->op_begin() + IdxVal,
>                                                          NumElems));
>  
> Note how we peak through a bitcast, but then use the original type for much of these calculations. So if the element size changed, we break (in this case, a bitcast from v2f32 to v4f16).
>  
> —escha
>  
> On Aug 28, 2017, at 8:28 AM, Craig Topper via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>  
> Author: ctopper
> Date: Mon Aug 28 08:28:33 2017
> New Revision: 311892
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=311892&view=rev <http://llvm.org/viewvc/llvm-project?rev=311892&view=rev>
> Log:
> [DAGCombiner] Teach visitEXTRACT_SUBVECTOR to turn extracts of BUILD_VECTOR into smaller BUILD_VECTORs
> 
> Only do this before operations are legalized of BUILD_VECTOR is Legal for the target.
> 
> Differential Revision: https://reviews.llvm.org/D37186 <https://reviews.llvm.org/D37186>
> 
> Modified:
>    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>    llvm/trunk/test/CodeGen/X86/fold-vector-sext-zext.ll
>    llvm/trunk/test/CodeGen/X86/widen_extract-1.ll
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=311892&r1=311891&r2=311892&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=311892&r1=311891&r2=311892&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Aug 28 08:28:33 2017
> @@ -15157,6 +15157,29 @@ SDValue DAGCombiner::visitEXTRACT_SUBVEC
>   // Skip bitcasting
>   V = peekThroughBitcast(V);
> 
> +  // If the input is a build vector. Try to make a smaller build vector.
> +  if (V->getOpcode() == ISD::BUILD_VECTOR) {
> +    if (auto *Idx = dyn_cast<ConstantSDNode>(N->getOperand(1))) {
> +      EVT InVT = V->getValueType(0);
> +      unsigned NumElems = NVT.getSizeInBits() / InVT.getScalarSizeInBits();
> +      if (NumElems > 0) {
> +        EVT ExtractVT = EVT::getVectorVT(*DAG.getContext(),
> +                                         InVT.getVectorElementType(), NumElems);
> +        if (!LegalOperations ||
> +            TLI.isOperationLegal(ISD::BUILD_VECTOR, ExtractVT)) {
> +          unsigned IdxVal = Idx->getZExtValue() * NVT.getScalarSizeInBits() /
> +                            InVT.getScalarSizeInBits();
> +
> +          // Extract the pieces from the original build_vector.
> +          SDValue BuildVec = DAG.getBuildVector(ExtractVT, SDLoc(N),
> +                                            makeArrayRef(V->op_begin() + IdxVal,
> +                                                         NumElems));
> +          return DAG.getBitcast(NVT, BuildVec);
> +        }
> +      }
> +    }
> +  }
> +
>   if (V->getOpcode() == ISD::INSERT_SUBVECTOR) {
>     // Handle only simple case where vector being inserted and vector
>     // being extracted are of same size.
> 
> Modified: llvm/trunk/test/CodeGen/X86/fold-vector-sext-zext.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fold-vector-sext-zext.ll?rev=311892&r1=311891&r2=311892&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fold-vector-sext-zext.ll?rev=311892&r1=311891&r2=311892&view=diff>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/fold-vector-sext-zext.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/fold-vector-sext-zext.ll Mon Aug 28 08:28:33 2017
> @@ -83,8 +83,7 @@ define <4 x i32> @test_sext_4i8_4i32_und
> define <4 x i64> @test_sext_4i8_4i64() {
> ; X32-LABEL: test_sext_4i8_4i64:
> ; X32:       # BB#0:
> -; X32-NEXT:    vmovaps {{.*#+}} xmm0 = [0,0,4294967295,4294967295]
> -; X32-NEXT:    vinsertf128 $1, {{\.LCPI.*}}, %ymm0, %ymm0
> +; X32-NEXT:    vmovaps {{.*#+}} ymm0 = [0,0,4294967295,4294967295,2,0,4294967293,4294967295]
> ; X32-NEXT:    retl
> ;
> ; X64-LABEL: test_sext_4i8_4i64:
> @@ -102,8 +101,7 @@ define <4 x i64> @test_sext_4i8_4i64() {
> define <4 x i64> @test_sext_4i8_4i64_undef() {
> ; X32-LABEL: test_sext_4i8_4i64_undef:
> ; X32:       # BB#0:
> -; X32-NEXT:    vpcmpeqd %xmm0, %xmm0, %xmm0
> -; X32-NEXT:    vinsertf128 $1, {{\.LCPI.*}}, %ymm0, %ymm0
> +; X32-NEXT:    vmovaps {{.*#+}} ymm0 = <u,u,4294967295,4294967295,u,u,4294967293,4294967295>
> ; X32-NEXT:    retl
> ;
> ; X64-LABEL: test_sext_4i8_4i64_undef:
> @@ -245,8 +243,7 @@ define <4 x i32> @test_zext_4i8_4i32() {
> define <4 x i64> @test_zext_4i8_4i64() {
> ; X32-LABEL: test_zext_4i8_4i64:
> ; X32:       # BB#0:
> -; X32-NEXT:    vmovaps {{.*#+}} xmm0 = [0,0,255,0]
> -; X32-NEXT:    vinsertf128 $1, {{\.LCPI.*}}, %ymm0, %ymm0
> +; X32-NEXT:    vmovaps {{.*#+}} ymm0 = [0,0,255,0,2,0,253,0]
> ; X32-NEXT:    retl
> ;
> ; X64-LABEL: test_zext_4i8_4i64:
> @@ -300,10 +297,7 @@ define <4 x i32> @test_zext_4i8_4i32_und
> define <4 x i64> @test_zext_4i8_4i64_undef() {
> ; X32-LABEL: test_zext_4i8_4i64_undef:
> ; X32:       # BB#0:
> -; X32-NEXT:    vmovaps {{.*#+}} xmm0 = <u,u,255,0>
> -; X32-NEXT:    movl $2, %eax
> -; X32-NEXT:    vmovd %eax, %xmm1
> -; X32-NEXT:    vinsertf128 $1, %xmm1, %ymm0, %ymm0
> +; X32-NEXT:    vmovaps {{.*#+}} ymm0 = <u,u,255,0,2,0,u,u>
> ; X32-NEXT:    retl
> ;
> ; X64-LABEL: test_zext_4i8_4i64_undef:
> 
> Modified: llvm/trunk/test/CodeGen/X86/widen_extract-1.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/widen_extract-1.ll?rev=311892&r1=311891&r2=311892&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/widen_extract-1.ll?rev=311892&r1=311891&r2=311892&view=diff>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/widen_extract-1.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/widen_extract-1.ll Mon Aug 28 08:28:33 2017
> @@ -7,8 +7,8 @@
> define void @convert(<2 x double>* %dst.addr, <3 x double> %src)  {
> ; X32-LABEL: convert:
> ; X32:       # BB#0: # %entry
> -; X32-NEXT:    movups {{[0-9]+}}(%esp), %xmm0
> ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
> +; X32-NEXT:    movups {{[0-9]+}}(%esp), %xmm0
> ; X32-NEXT:    movaps %xmm0, (%eax)
> ; X32-NEXT:    retl
> ;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170831/5f161b48/attachment.html>


More information about the llvm-commits mailing list