Skip to content

[LLVM][DAGCombiner] Improve simplifyDivRem's effectiveness after type legalisation. #162706

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

Merged
merged 2 commits into from
Oct 14, 2025
Merged

[LLVM][DAGCombiner] Improve simplifyDivRem's effectiveness after type legalisation. #162706

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63af232
[LLVM][DAGCombiner] Improve simplifyDivRem's effectiveness after type…
paulwalker-arm Oct 9, 2025
a3e7df8
Use isOneOrOneSplat and name IR test variables.
paulwalker-arm Oct 10, 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
2 changes: 1 addition & 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 @@ -5042,7 +5042,7 @@ static SDValue simplifyDivRem(SDNode *N, SelectionDAG &DAG) {

unsigned Opc = N->getOpcode();
bool IsDiv = (ISD::SDIV == Opc) || (ISD::UDIV == Opc);
ConstantSDNode *N1C = isConstOrConstSplat(N1);
ConstantSDNode *N1C = isConstOrConstSplat(N1, false, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing as N1C only has one use, I'm wondering if it's maybe nicer to replace the (single) use below with isOneOrOneSplat, which already allows truncation?

-  if ((N1C && N1C->isOne()) || (VT.getScalarType() == MVT::i1))
+  if (isOneOrOneSplat(N1) || (VT.getScalarType() == MVT::i1))

https://github.com/llvm/llvm-project/blob/5cef6f38c0c7a59d8ce0854d04613d8606326023/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L5077

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea what these arguments are, comment them or use some named wrapper?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've followed @hazzlim's suggestion to use isOneOrOneSplat and remove this line.


// X / undef -> undef
// X % undef -> undef
Expand Down
85 changes: 85 additions & 0 deletions llvm/test/CodeGen/AArch64/combine-sdiv.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1774,3 +1774,88 @@ define i128 @combine_i128_sdiv_const100(i128 %x) {
%1 = sdiv i128 %x, 100
ret i128 %1
}

; The following only becomes an sdiv_by_one after type legalisation, after which
; the splatted scalar constant has a different type to the splat vector. This
; test verifies DAGCombiner does not care about this type difference.
define <16 x i16> @combine_vec_sdiv_by_one_obfuscated(<16 x i16> %x) "target-features"="+sve" {
; CHECK-SD-LABEL: combine_vec_sdiv_by_one_obfuscated:
; CHECK-SD: // %bb.0:
; CHECK-SD-NEXT: ret
;
; CHECK-GI-LABEL: combine_vec_sdiv_by_one_obfuscated:
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as an aside, GISel really doesn't do a great job here 🫠

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't have any of these combines

; CHECK-GI: // %bb.0:
; CHECK-GI-NEXT: movi v2.2d, #0000000000000000
; CHECK-GI-NEXT: movi v3.8h, #1
; CHECK-GI-NEXT: smov w8, v0.h[0]
; CHECK-GI-NEXT: mov v3.h[0], v2.h[0]
; CHECK-GI-NEXT: smov w9, v3.h[0]
; CHECK-GI-NEXT: smov w16, v3.h[7]
; CHECK-GI-NEXT: sdiv w14, w8, w9
; CHECK-GI-NEXT: smov w8, v0.h[1]
; CHECK-GI-NEXT: smov w9, v3.h[1]
; CHECK-GI-NEXT: sdiv w15, w8, w9
; CHECK-GI-NEXT: smov w8, v0.h[2]
; CHECK-GI-NEXT: smov w9, v3.h[2]
; CHECK-GI-NEXT: sdiv w13, w8, w9
; CHECK-GI-NEXT: smov w8, v0.h[3]
; CHECK-GI-NEXT: smov w9, v3.h[3]
; CHECK-GI-NEXT: sdiv w12, w8, w9
; CHECK-GI-NEXT: smov w8, v0.h[4]
; CHECK-GI-NEXT: smov w9, v3.h[4]
; CHECK-GI-NEXT: sdiv w11, w8, w9
; CHECK-GI-NEXT: smov w8, v0.h[5]
; CHECK-GI-NEXT: smov w9, v3.h[5]
; CHECK-GI-NEXT: sdiv w10, w8, w9
; CHECK-GI-NEXT: smov w8, v0.h[6]
; CHECK-GI-NEXT: smov w9, v3.h[6]
; CHECK-GI-NEXT: movi v3.8h, #1
; CHECK-GI-NEXT: smov w17, v3.h[0]
; CHECK-GI-NEXT: smov w18, v3.h[1]
; CHECK-GI-NEXT: smov w0, v3.h[2]
; CHECK-GI-NEXT: smov w1, v3.h[3]
; CHECK-GI-NEXT: smov w2, v3.h[4]
; CHECK-GI-NEXT: smov w3, v3.h[5]
; CHECK-GI-NEXT: sdiv w8, w8, w9
; CHECK-GI-NEXT: smov w9, v0.h[7]
; CHECK-GI-NEXT: fmov s0, w14
; CHECK-GI-NEXT: mov v0.h[1], w15
; CHECK-GI-NEXT: smov w15, v1.h[6]
; CHECK-GI-NEXT: mov v0.h[2], w13
; CHECK-GI-NEXT: sdiv w9, w9, w16
; CHECK-GI-NEXT: smov w16, v1.h[0]
; CHECK-GI-NEXT: mov v0.h[3], w12
; CHECK-GI-NEXT: smov w12, v1.h[7]
; CHECK-GI-NEXT: mov v0.h[4], w11
; CHECK-GI-NEXT: sdiv w16, w16, w17
; CHECK-GI-NEXT: smov w17, v1.h[1]
; CHECK-GI-NEXT: mov v0.h[5], w10
; CHECK-GI-NEXT: mov v0.h[6], w8
; CHECK-GI-NEXT: sdiv w17, w17, w18
; CHECK-GI-NEXT: smov w18, v1.h[2]
; CHECK-GI-NEXT: fmov s2, w16
; CHECK-GI-NEXT: smov w16, v3.h[6]
; CHECK-GI-NEXT: mov v0.h[7], w9
; CHECK-GI-NEXT: sdiv w18, w18, w0
; CHECK-GI-NEXT: smov w0, v1.h[3]
; CHECK-GI-NEXT: mov v2.h[1], w17
; CHECK-GI-NEXT: sdiv w0, w0, w1
; CHECK-GI-NEXT: smov w1, v1.h[4]
; CHECK-GI-NEXT: mov v2.h[2], w18
; CHECK-GI-NEXT: sdiv w1, w1, w2
; CHECK-GI-NEXT: smov w2, v1.h[5]
; CHECK-GI-NEXT: mov v2.h[3], w0
; CHECK-GI-NEXT: sdiv w14, w2, w3
; CHECK-GI-NEXT: mov v2.h[4], w1
; CHECK-GI-NEXT: sdiv w13, w15, w16
; CHECK-GI-NEXT: smov w15, v3.h[7]
; CHECK-GI-NEXT: mov v2.h[5], w14
; CHECK-GI-NEXT: sdiv w10, w12, w15
; CHECK-GI-NEXT: mov v2.h[6], w13
; CHECK-GI-NEXT: mov v2.h[7], w10
; CHECK-GI-NEXT: mov v1.16b, v2.16b
; CHECK-GI-NEXT: ret
%1 = shufflevector <16 x i16> zeroinitializer, <16 x i16> splat (i16 1), <16 x i32> <i32 0, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31>
Copy link
Contributor

Choose a reason for hiding this comment

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

Use named values in tests

%2 = sdiv <16 x i16> %x, %1
ret <16 x i16> %2
}