[DAGCombiner] Treat extracts from build_vectors that are splats as free #65773
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds tests for fixed and scalable vectors where we have a binary op on two splats that could be scalarized. Normally this would be scalarized in the middle-end by VectorCombine, but as noted in https://reviews.llvm.org/D159190, this pattern can crop up during CodeGen afterwards. Note that a combine already exists for this, but currently it only works on scalable vectors where the element type == XLEN. See llvm#65068 and llvm#65072
cor3ntin
added
llvm:SelectionDAG
When scalarizing bin ops of splats, we treat the extract as free for a splat_vector because its operand is already scalar, i.e. (extract idx, (splat_vector x)) -> x. The same also applies for a build_vector that's a splat: (extract idx, (build_vector x x x x)) -> x. This patch takes this into account, which enables scalarization for fixed length vectors, since the current canonical form for a splatted fixed length vector is still build_vector. This improves what we were seeing on RISC-V in llvm#65068, but unfortunately causes some patterns to be missed on other targets. One big one is that on AArch64 and X86 scalarizing (xor (splat x), (splat -1)) to (splat (xor x, -1)) prevents vnot from being matched, which for example prevents bif from being matched. Posting this patch as a WIP to show my findings.
lukel97
force-pushed
the
scalarize-binop-of-splats-build-vector/free
branch
from
59caba7
to
7980f5b
Compare
arsenm
reviewed
Sep 13, 2023
| @@ -244,7 +244,8 @@ define void @undef_hi_op_v2f16(half %arg0) { | |||
| ; GFX9-LABEL: undef_hi_op_v2f16: | |||
| ; GFX9: ; %bb.0: | |||
| ; GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) | |||
| ; GFX9-NEXT: v_pk_add_f16 v0, v0, 1.0 op_sel_hi:[1,0] | |||
| ; GFX9-NEXT: v_add_f16_e32 v0, 1.0, v0 | |||
| ; GFX9-NEXT: v_pack_b32_f16 v0, v0, v0 | |||
There was a problem hiding this comment.
Yeah, I've marked this patch as a draft since I just want to share my findings. Hoping others might have some ideas on additional patterns that can be matched to mitigate all these regressions
| ; SI-NEXT: v_mov_b32_e32 v0, s4 | ||
| ; SI-NEXT: v_alignbit_b32 v0, s5, v0, 16 |
| ; GFX9-NEXT: v_pk_ashrrev_i16 v0, 15, v5 op_sel_hi:[0,0] | ||
| ; GFX9-NEXT: s_movk_i32 s4, 0x8000 | ||
| ; GFX9-NEXT: v_ashrrev_i16_e32 v0, 15, v5 | ||
| ; GFX9-NEXT: s_mov_b32 s4, 0x5040100 |
lukel97
changed the title
|
We already have several places where we early-out if we'd break a NOT pattern |
|
@lukel97 reverse-ping - are you still looking at this? |
No, feel free to take over or close this |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When scalarizing bin ops of splats, we treat the extract as free for a
splat_vector because its operand is already scalar, i.e.
(extract idx, (splat_vector x)) -> x.
The same also applies for a build_vector that's a splat:
(extract idx, (build_vector x x x x)) -> x.
This patch takes this into account, which enables scalarization for fixed
length vectors, since the current canonical form for a splatted fixed length
vector is still build_vector.
This improves what we were seeing on RISC-V in #65068, but unfortunately causes
some patterns to be missed on other targets. One big one is that on AArch64
and X86 scalarizing (xor (splat x), (splat -1)) to (splat (xor x, -1)) prevents
vnot from being matched, which for example prevents bif from being matched.
Posting this patch as a WIP to show my findings.