[PATCH] D109065: [X86] combineX86ShufflesRecursively(): call SimplifyMultipleUseDemandedVectorElts() on after finishing recursing

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 18 03:22:25 PDT 2021


lebedev.ri added inline comments.


================
Comment at: llvm/test/CodeGen/X86/oddshuffles.ll:2268
+; AVX1-NEXT:    vblendps {{.*#+}} ymm0 = ymm1[0],ymm0[1],ymm1[2,3,4,5,6,7]
+; AVX1-NEXT:    vmovd %eax, %xmm2
+; AVX1-NEXT:    vpshufd {{.*#+}} xmm2 = xmm2[0,0,0,0]
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > RKSimon wrote:
> > > Looks like we're missing a fold to share scalar_to_vector(x) and scalar_to_vector(trunc(x)) (maybe worth supporting scalar_to_vector(ext(x)) as well)?
> > This stuff is broken.
> > We've in AVX1-more, and only have broadcast-from-mem,
> > yet we've successfully obscured the load via the truncation.
> > 
> > I suppose we could look past ext/trunc of scalar_to_vector operand,
> > and change it to bitcast/ext of scalar_to_vector itself,
> > let me see.
> > 
> > ```
> > Optimized legalized selection DAG: %bb.0 'splat_v3i32:'
> > SelectionDAG has 28 nodes:
> >   t0: ch = EntryToken
> >     t2: i64,ch = CopyFromReg t0, Register:i64 %0
> >   t27: i64,ch = load<(load (s64) from %ir.ptr, align 1)> t0, t2, undef:i64
> >       t24: v8i32 = BUILD_VECTOR Constant:i32<0>, undef:i32, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>
> >           t30: v2i64 = scalar_to_vector t27
> >         t107: v4i64 = insert_subvector undef:v4i64, t30, Constant:i64<0>
> >       t108: v8i32 = bitcast t107
> >     t101: v8i32 = X86ISD::BLENDI t24, t108, TargetConstant:i8<2>
> >   t19: ch,glue = CopyToReg t0, Register:v8i32 $ymm0, t101
> >       t25: v8i32 = BUILD_VECTOR Constant:i32<0>, Constant:i32<0>, undef:i32, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>
> >           t91: i32 = truncate t27
> >         t92: v4i32 = X86ISD::VBROADCAST t91
> >       t94: v8i32 = insert_subvector undef:v8i32, t92, Constant:i64<0>
> >     t97: v8i32 = X86ISD::BLENDI t25, t94, TargetConstant:i8<4>
> >   t21: ch,glue = CopyToReg t19, Register:v8i32 $ymm1, t97, t19:1
> >   t22: ch = X86ISD::RET_FLAG t21, TargetConstant:i32<0>, Register:v8i32 $ymm0, Register:v8i32 $ymm1, t21:1
> > ```
> While something like this should work:
> ```
> diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index 5a49f33e46fe..4d7c2c2a8651 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -21824,6 +21824,13 @@ SDValue DAGCombiner::visitSCALAR_TO_VECTOR(SDNode *N) {
>      }
>    }
>  
> +  // Fold SCALAR_TO_VECTOR(TRUNCATE(V)) to SCALAR_TO_VECTOR(V),
> +  // by making trucation of the operand implicit.
> +  if (InVal.getOpcode() == ISD::TRUNCATE && VT.isFixedLengthVector() &&
> +      Level < AfterLegalizeDAG)
> +    return DAG.getNode(ISD::SCALAR_TO_VECTOR, SDLoc(N), VT,
> +                       InVal->getOperand(0));
> +
>    return SDValue();
>  }
>  
> diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
> index 09ba7af6e38a..695cc8303cc1 100644
> --- a/llvm/lib/Target/X86/X86ISelLowering.cpp
> +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
> @@ -14076,10 +14076,12 @@ static SDValue lowerShuffleAsBroadcast(const SDLoc &DL, MVT VT, SDValue V1,
>  
>    // If this is a scalar, do the broadcast on this type and bitcast.
>    if (!V.getValueType().isVector()) {
> -    assert(V.getScalarValueSizeInBits() == NumEltBits &&
> -           "Unexpected scalar size");
> -    MVT BroadcastVT = MVT::getVectorVT(V.getSimpleValueType(),
> -                                       VT.getVectorNumElements());
> +    if(V.getValueType().isInteger() &&
> +       V.getScalarValueSizeInBits() > NumEltBits)
> +      V = DAG.getNode(ISD::TRUNCATE, DL, VT.getScalarType(), V);
> +    assert(V.getScalarValueSizeInBits() == NumEltBits && "Unexpected scalar size");
> +    MVT BroadcastVT =
> +        MVT::getVectorVT(V.getSimpleValueType(), VT.getVectorNumElements());
>      return DAG.getBitcast(VT, DAG.getNode(Opcode, DL, BroadcastVT, V));
>    }
> ```
> it doesn't catch anything with the cut-off,
> and without it. it exposes numerous places that don't expect this truncation to be implicit.
> 
I've looked again, and i'm not sure i have enough motivation to tackle all the fallout from the
`scalar_to_vector(trunc(x)) --> scalar_to_vector(x)` fold, unless i misunderstood the suggestion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109065/new/

https://reviews.llvm.org/D109065



More information about the llvm-commits mailing list