Skip to content

[DAGCombiner] Don't fold cheap extracts of multiple use splats #134120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[DAGCombiner] Don't fold cheap extracts of multiple use splats #134120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9712bc6
[DAGCombiner] Don't fold cheap extracts of multiple use splats
lukel97 Apr 2, 2025
0a9041d
Workaround AArch64 regressions
lukel97 Apr 2, 2025
e0f4ea3
Demorgan
lukel97 Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25382,7 +25382,9 @@ SDValue DAGCombiner::visitEXTRACT_SUBVECTOR(SDNode *N) {

// ty1 extract_vector(ty2 splat(V))) -> ty1 splat(V)
if (V.getOpcode() == ISD::SPLAT_VECTOR)
if (DAG.isConstantValueOfAnyType(V.getOperand(0)) || V.hasOneUse())
if ((DAG.isConstantValueOfAnyType(V.getOperand(0)) &&
!TLI.isExtractSubvectorCheap(NVT, V.getValueType(), ExtIdx)) ||
V.hasOneUse())
if (!LegalOperations || TLI.isOperationLegal(ISD::SPLAT_VECTOR, NVT))
return DAG.getSplatVector(NVT, DL, V.getOperand(0));

Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20223,17 +20223,18 @@ performExtractSubvectorCombine(SDNode *N, TargetLowering::DAGCombinerInfo &DCI,
return SDValue();

EVT VT = N->getValueType(0);
if (!VT.isScalableVector() || VT.getVectorElementType() != MVT::i1)
return SDValue();

SDValue V = N->getOperand(0);

if (VT.isScalableVector() != V->getValueType(0).isScalableVector())
return SDValue();
Comment on lines +20228 to +20229
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed to prevent an infinite loop where fixed length splats got legalized to scalable + an fixed extract

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a explanatory comment to explain this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please.


// NOTE: This combine exists in DAGCombiner, but that version's legality check
// blocks this combine because the non-const case requires custom lowering.
// We also want to perform it even when the splat has multiple uses.
//
// ty1 extract_vector(ty2 splat(const))) -> ty1 splat(const)
if (V.getOpcode() == ISD::SPLAT_VECTOR)
if (isa<ConstantSDNode>(V.getOperand(0)))
if (isa<ConstantSDNode, ConstantFPSDNode>(V.getOperand(0)))
return DAG.getNode(ISD::SPLAT_VECTOR, SDLoc(N), VT, V.getOperand(0));

return SDValue();
Expand Down
18 changes: 6 additions & 12 deletions llvm/test/CodeGen/RISCV/rvv/insertelt-int-rv64.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,13 +761,10 @@ define <vscale x 8 x i64> @insertelt_nxv8i64_idx(<vscale x 8 x i64> %v, i64 %elt
define <vscale x 4 x i32> @insertelt_nxv4i32_zeroinitializer_0(i32 %x) {
; CHECK-LABEL: insertelt_nxv4i32_zeroinitializer_0:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a1, zero, e32, m1, ta, ma
; CHECK-NEXT: vmv.v.i v10, 0
; CHECK-NEXT: vsetvli zero, zero, e32, m1, tu, ma
; CHECK-NEXT: vmv.s.x v10, a0
; CHECK-NEXT: vsetvli a0, zero, e32, m2, ta, ma
; CHECK-NEXT: vsetvli a1, zero, e32, m2, ta, ma
; CHECK-NEXT: vmv.v.i v8, 0
; CHECK-NEXT: vmv1r.v v8, v10
; CHECK-NEXT: vsetvli zero, zero, e32, m2, tu, ma
; CHECK-NEXT: vmv.s.x v8, a0
; CHECK-NEXT: ret
%v = insertelement <vscale x 4 x i32> zeroinitializer, i32 %x, i64 0
ret <vscale x 4 x i32> %v
Expand All @@ -776,14 +773,11 @@ define <vscale x 4 x i32> @insertelt_nxv4i32_zeroinitializer_0(i32 %x) {
define <vscale x 4 x i32> @insertelt_imm_nxv4i32_zeroinitializer_0(i32 %x) {
; CHECK-LABEL: insertelt_imm_nxv4i32_zeroinitializer_0:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a0, zero, e32, m1, ta, ma
; CHECK-NEXT: vmv.v.i v10, 0
; CHECK-NEXT: li a0, 42
; CHECK-NEXT: vsetvli zero, zero, e32, m1, tu, ma
; CHECK-NEXT: vmv.s.x v10, a0
; CHECK-NEXT: vsetvli a0, zero, e32, m2, ta, ma
; CHECK-NEXT: vmv.v.i v8, 0
; CHECK-NEXT: vmv1r.v v8, v10
; CHECK-NEXT: li a0, 42
; CHECK-NEXT: vsetvli zero, zero, e32, m2, tu, ma
; CHECK-NEXT: vmv.s.x v8, a0
; CHECK-NEXT: ret
%v = insertelement <vscale x 4 x i32> zeroinitializer, i32 42, i64 0
ret <vscale x 4 x i32> %v
Expand Down
Loading