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

paulwalker-arm · 2025-10-09T17:30:10Z

simplifyDivRem does not work as well after type legalisation because splatted constants can have a size mismatch between the scalar to splat and the element type of the splatted result. simplifyDivRem does not seem to care about this mismatch so I've updated the isConstOrConstSplat call for the divisor to allow truncation.

This comes from investigating #162616. I have a separate fix for the incorrect lowering but I figure this should not have got through to lowering in the first place.

… legalisation. simplifyDivRem does not work as well after type legalsiation because splatted constants can have a size mismatch between the scalar to splat and the element type of the splatted result. simplifyDivRem does not seem to care about this mismatch so I've updated the isConstOrConstSplat call for the divisor to allow truncation.

llvmbot · 2025-10-09T17:30:42Z

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-aarch64

Author: Paul Walker (paulwalker-arm)

Changes

simplifyDivRem does not work as well after type legalsiation because splatted constants can have a size mismatch between the scalar to splat and the element type of the splatted result. simplifyDivRem does not seem to care about this mismatch so I've updated the isConstOrConstSplat call for the divisor to allow truncation.

This comes from investigating #162616. I have a separate fix for the incorrect lowering but I figure this should not have got through to lowering in the first place.

Full diff: https://github.com/llvm/llvm-project/pull/162706.diff

2 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+1-1)
(modified) llvm/test/CodeGen/AArch64/combine-sdiv.ll (+85)

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index c5c38661f1d71..b765ccee5bca3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -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);
 
   // X / undef -> undef
   // X % undef -> undef
diff --git a/llvm/test/CodeGen/AArch64/combine-sdiv.ll b/llvm/test/CodeGen/AArch64/combine-sdiv.ll
index dc88f9414b866..b27f9d51064d4 100644
--- a/llvm/test/CodeGen/AArch64/combine-sdiv.ll
+++ b/llvm/test/CodeGen/AArch64/combine-sdiv.ll
@@ -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:
+; 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>
+  %2 = sdiv <16 x i16> %x, %1
+  ret <16 x i16> %2
+}

hazzlim · 2025-10-09T17:50:01Z

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

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


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

hazzlim · 2025-10-09T17:55:17Z

llvm/test/CodeGen/AArch64/combine-sdiv.ll

+; CHECK-SD:       // %bb.0:
+; CHECK-SD-NEXT:    ret
+;
+; CHECK-GI-LABEL: combine_vec_sdiv_by_one_obfuscated:


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

It doesn't have any of these combines

hazzlim · 2025-10-09T18:02:45Z

simplifyDivRem does not work as well after type legalsiation because splatted constants can have a size mismatch between the scalar to splat and the element type of the splatted result. simplifyDivRem does not seem to care about this mismatch so I've updated the isConstOrConstSplat call for the divisor to allow truncation.

This comes from investigating #162616. I have a separate fix for the incorrect lowering but I figure this should not have got through to lowering in the first place.

Commit message typo nit: legalsiation -> legalization/legalisation

arsenm · 2025-10-10T03:42:33Z

llvm/test/CodeGen/AArch64/combine-sdiv.ll

+; 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>


Use named values in tests

arsenm · 2025-10-10T03:43:09Z

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

@@ -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);


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

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

hazzlim

LGTM! :)

llvmbot added backend:AArch64 llvm:SelectionDAG SelectionDAGISel as well labels Oct 9, 2025

paulwalker-arm mentioned this pull request Oct 9, 2025

[LLVM][CodeGen][SVE] ASRD cannot represent sdiv-by-one. #162708

Merged

hazzlim reviewed Oct 9, 2025

View reviewed changes

arsenm reviewed Oct 10, 2025

View reviewed changes

Use isOneOrOneSplat and name IR test variables.

a3e7df8

hazzlim approved these changes Oct 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

paulwalker-arm commented Oct 9, 2025 •

edited

Loading

Uh oh!

llvmbot commented Oct 9, 2025 •

edited

Loading

Uh oh!

hazzlim Oct 9, 2025

Uh oh!

hazzlim Oct 9, 2025

Uh oh!

arsenm Oct 10, 2025

Uh oh!

hazzlim commented Oct 9, 2025

Uh oh!

arsenm Oct 10, 2025

Uh oh!

arsenm Oct 10, 2025

Uh oh!

paulwalker-arm Oct 10, 2025

Uh oh!

hazzlim left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

paulwalker-arm commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hazzlim Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

hazzlim Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

hazzlim commented Oct 9, 2025

Uh oh!

arsenm Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

paulwalker-arm Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

hazzlim left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

paulwalker-arm commented Oct 9, 2025 •

edited

Loading

llvmbot commented Oct 9, 2025 •

edited

Loading