[X86] SimplifyDemandedVectorEltsForTargetNode - reduce the size of VPERMV/VPERMV3 nodes if the upper elements are not demanded #133923
Conversation
| SmallVector<SDValue, 2> Ops; | ||
| // TODO: Handle 128-bit PERMD/Q -> PSHUFD | ||
| if (Subtarget.hasVLX() && | ||
| (VT.is512BitVector() || VT.getScalarSizeInBits() <= 16) && |
There was a problem hiding this comment.
Why VT.getScalarSizeInBits() <= 16? I don't see a test for it.
There was a problem hiding this comment.
OK - I'll try to add test coverage.
There was a problem hiding this comment.
I've added test coverage and handling for 128-bit 32/64element shuffles to correctly use VPERMILPS/D instructions (which can then further simplify to VPSHUFD etc.)
There was a problem hiding this comment.
Do you mean 2426ac6? I don't see the test in this patch?
There was a problem hiding this comment.
Yes, the new VPERMILP code path is now being hit by the 2426ac6 test - but it still eventually folds to the same VSHUFPS as it did before, just via a different set of combines.
There was a problem hiding this comment.
I see the point.
The initial question is about the VT.getScalarSizeInBits() <= 16, where I expect to see some change on vpermi2b/w that don't have 512-bit size, but I didn't find them in tests.
| if (all_of(Mask, | ||
| [&](int M) { return isUndefOrInRange(M, 0, HalfElts); })) { |
There was a problem hiding this comment.
Where did we check the upper elements are not demanded?
There was a problem hiding this comment.
At about line 43714:
// For 256/512-bit ops that are 128/256-bit ops glued together, if we do not
// demand any of the high elements, then narrow the op to 128/256-bits: e.g.
// (op ymm0, ymm1) --> insert undef, (op xmm0, xmm1), 0
if ((VT.is256BitVector() || VT.is512BitVector()) &&
DemandedElts.lshr(NumElts / 2) == 0) {
unsigned SizeInBits = VT.getSizeInBits();
unsigned ExtSizeInBits = SizeInBits / 2;
// See if 512-bit ops only use the bottom 128-bits.
if (VT.is512BitVector() && DemandedElts.lshr(NumElts / 4) == 0)
ExtSizeInBits = SizeInBits / 4;These "vector width reduction" folds are after the standard SimplifyDemandedVectorElts simplifications that are handled earlier in SimplifyDemandedVectorEltsForTargetNode .
Based off #133923 - test to ensure the VPERMV node as only the lower 128-bit source elements are demanded.
RKSimon
force-pushed
the
x86-demandedelts-vpermv-vpermv3
branch
from
09f9845 to
1070f6b
Compare
| ; AVX512VBMI-FAST-NEXT: vmovdqa {{.*#+}} xmm1 = [0,4,8,12,16,20,24,28,32,36,40,44,48,52,56,79] | ||
| ; AVX512VBMI-FAST-NEXT: vpxor %xmm2, %xmm2, %xmm2 | ||
| ; AVX512VBMI-FAST-NEXT: vmovdqa {{.*#+}} xmm1 = [64,65,66,67,68,69,24,28,32,36,40,44,48,52,56,79] | ||
| ; AVX512VBMI-FAST-NEXT: vpmovdb %ymm0, %xmm2 |
There was a problem hiding this comment.
This is not expected either, right?
There was a problem hiding this comment.
This change allows the VPMOVDB node to be created from another VPERMV3 node, so we no longer have 2 VPERMV3 nodes that we can easily fold together. We're still struggling to combine shuffles across different vector widths - it will be handled eventually after #133947 but that is a much larger WIP patch that is highly dependent on us getting this in first.......
… VPERMV3 canonicalization Pulled out of #133923 - this prevents regressions with SimplifyDemandedVectorEltsForTargetNode exposing VPERMV3(X,M,X) repeated operand patterns which were getting concatenated to wider VPERMV nodes before simpler canonicalizations could clean them up.
…ERMV/VPERMV3 nodes if the upper elements are not demanded With AVX512VL targets, use 128/256-bit VPERMV/VPERMV3 nodes when we only need the lower elements. This exposed an issue with VPERMV3(X,M,Y) -> VPERMV(M,CONCAT(X,Y)) folds when X==Y, so I had to move that fold after the other VPERMV3 folds/canonicalizations. I also took the opportunity to try to support the VPERMV(M,CONCAT(Y,X)) case as well, but we can revert this if we'd prefer to avoid the extra VSHUFF64X2 node for non-constant shuffle masks (but separate loads) instead.
RKSimon
force-pushed
the
x86-demandedelts-vpermv-vpermv3
branch
from
5ac2a34 to
6d06e67
Compare
…duce the size of VPERMV/VPERMV3 nodes if the upper elements are not demanded" (#134256) Found a typo in the VPERMV3 mask adjustment - I'm going to revert and re-apply the patch with a fix Reverts llvm/llvm-project#133923
…ERMV/VPERMV3 nodes if the upper elements are not demanded (REAPPLIED) With AVX512VL targets, use 128/256-bit VPERMV/VPERMV3 nodes when we only need the lower elements. Reapplied version of llvm#133923 with fix for typo in the VPERMV3 mask adjustment
…ERMV v16f32/v16i32 nodes if the upper elements are not demanded Missed in llvm#133923 - even without AVX512VL, we can replace VPERMV v16f32/v16i32 nodes with the AVX2 v8f32/v8i32 equivalents.
…ERMV v16f32/v16i32 nodes if the upper elements are not demanded (llvm#134890) Missed in llvm#133923 - even without AVX512VL, we can replace VPERMV v16f32/v16i32 nodes with the AVX2 v8f32/v8i32 equivalents.
…ERMV v16f32/v16i32 nodes if the upper elements are not demanded (llvm#134890) Missed in llvm#133923 - even without AVX512VL, we can replace VPERMV v16f32/v16i32 nodes with the AVX2 v8f32/v8i32 equivalents.
With AVX512VL targets, use 128/256-bit VPERMV/VPERMV3 nodes when we only need the lower elements.
This exposed an issue with VPERMV3(X,M,Y) -> VPERMV(M,CONCAT(X,Y)) folds when X==Y, so I had to move that fold after the other VPERMV3 folds/canonicalizations.
I also took the opportunity to try to improve support for the VPERMV(M,CONCAT(Y,X)) case as well, but we can revert this if we'd prefer to avoid the extra VSHUFF64X2 node for non-constant shuffle masks (but separate loads) instead.