[llvm] r311892 - [DAGCombiner] Teach visitEXTRACT_SUBVECTOR to turn extracts of BUILD_VECTOR into smaller BUILD_VECTORs
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 6 10:19:16 PDT 2017
I implemented your suggestion in r312621. Let me know if it doesn't fix
your issue.
~Craig
On Tue, Sep 5, 2017 at 1:54 PM, Justin Bogner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> "Topper, Craig via llvm-commits" <llvm-commits at lists.llvm.org> writes:
> > Hopefully fixed with r312253. My attempt at preventing element splits
> > wasn’t strong enough.
>
> It looks like this fixed the case that was reported here, but I'm still
> seeing a related problem where we get something like the below (using
> similar debug output to escha's example):
>
> Combining: t95: v2i8 = extract_subvector t74, Constant:i32<0>
> ---------
> t74: v4i8 = bitcast t73
> t73: v2i16 = BUILD_VECTOR t70, t72
> ExtractSize: 16, EltSize: 16, NumElems: 1
> LegalOperations: 0
> LegalTypes: 1
> isLegalOp: 0
> t151: v1i16 = BUILD_VECTOR t70
> ---------
> ... into: t152: v2i8 = bitcast t151
>
> Here we create a v1i16 (which is not a legal type), but we're past type
> legalization where we would scalarize this.
>
> I guess the easiest fix is to check LegalTypes/isTypeLegal much like we
> do with the operation already:
>
> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index ae923e8ac07..c068e1df836 100644
> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -15180,8 +15180,9 @@ SDValue DAGCombiner::visitEXTRACT_SUBVECTOR(SDNode*
> N) {
> unsigned NumElems = ExtractSize / EltSize;
> EVT ExtractVT = EVT::getVectorVT(*DAG.getContext(),
> InVT.getVectorElementType(),
> NumElems);
> - if (!LegalOperations ||
> - TLI.isOperationLegal(ISD::BUILD_VECTOR, ExtractVT)) {
> + if ((!LegalOperations ||
> + TLI.isOperationLegal(ISD::BUILD_VECTOR, ExtractVT)) &&
> + (!LegalTypes || TLI.isTypeLegal(ExtractVT))) {
> unsigned IdxVal = (Idx->getZExtValue() *
> NVT.getScalarSizeInBits()) /
> EltSize;
>
> WDYT?
>
> > From: escha at apple.com
> > Sent: Thursday, August 31, 2017 1:41 AM
> > 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
> >
> > 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<
> mailto: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: escha at apple.com<mailto:escha at apple.com>
> > Sent: Wednesday, August 30, 2017 3:05 PM
> > To: Topper, Craig <craig.topper at intel.com<mailto:craig.topper at intel.com>
> >
> > Cc: llvm-commits at lists.llvm.org<mailto: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
> > 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
> >
> > 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
> > ============================================================
> ==================
> >
> > --- 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
> > ============================================================
> ==================
> > --- 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
> > ============================================================
> ==================
> > --- 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
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> 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/20170906/a99a9df2/attachment.html>
More information about the llvm-commits
mailing list