Skip to content

Reapply "[AMDGPU] Always lower s/udiv64 by constant to MUL" #101942

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

Jump to bottom
Merged
merged 2 commits into from
Aug 12, 2024
Merged

Reapply "[AMDGPU] Always lower s/udiv64 by constant to MUL" #101942

merged 2 commits into from
Aug 12, 2024

Conversation

Pierre-vh
Copy link
Contributor

Reland #100723, fixing the ARM issue at the cost of a small regression in AMDGPU.

Solves #100383

@Pierre-vh
Copy link
Contributor Author

@MaskRay please check that this is good for Halide now. The test case is based on the reproducer given.
@davemgreen please check the ARM test case

@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2024 •
edited
Loading

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-llvm-selectiondag

Author: Pierre van Houtryve (Pierre-vh)

Changes

Reland #100723, fixing the ARM issue at the cost of a small regression in AMDGPU.

Solves #100383

Patch is 161.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101942.diff

9 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/TargetLowering.h (+2)
  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+14-2)
  • (modified) llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll (+338-1036)
  • (added) llvm/test/CodeGen/AMDGPU/div-rem-by-constant-64.ll (+1412)
  • (modified) llvm/test/CodeGen/AMDGPU/udiv.ll (+65-263)
  • (modified) llvm/test/CodeGen/AMDGPU/udiv64.ll (+42-140)
  • (modified) llvm/test/CodeGen/AMDGPU/urem64.ll (+31-71)
  • (added) llvm/test/CodeGen/ARM/div-by-constant-to-mul-crash.ll (+56)
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 9ccdbab008aec..bd394071d5d7f 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -5078,8 +5078,10 @@ class TargetLowering : public TargetLoweringBase {
   //
 
   SDValue BuildSDIV(SDNode *N, SelectionDAG &DAG, bool IsAfterLegalization,
+                    bool IsAfterLegalTypes,
                     SmallVectorImpl<SDNode *> &Created) const;
   SDValue BuildUDIV(SDNode *N, SelectionDAG &DAG, bool IsAfterLegalization,
+                    bool IsAfterLegalTypes,
                     SmallVectorImpl<SDNode *> &Created) const;
   // Build sdiv by power-of-2 with conditional move instructions
   SDValue buildSDIVPow2WithCMov(SDNode *N, const APInt &Divisor,
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 20b3ca21ef8a7..80a486cad27e3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -27805,7 +27805,7 @@ SDValue DAGCombiner::BuildSDIV(SDNode *N) {
     return SDValue();
 
   SmallVector<SDNode *, 8> Built;
-  if (SDValue S = TLI.BuildSDIV(N, DAG, LegalOperations, Built)) {
+  if (SDValue S = TLI.BuildSDIV(N, DAG, LegalOperations, LegalTypes, Built)) {
     for (SDNode *N : Built)
       AddToWorklist(N);
     return S;
@@ -27846,7 +27846,7 @@ SDValue DAGCombiner::BuildUDIV(SDNode *N) {
     return SDValue();
 
   SmallVector<SDNode *, 8> Built;
-  if (SDValue S = TLI.BuildUDIV(N, DAG, LegalOperations, Built)) {
+  if (SDValue S = TLI.BuildUDIV(N, DAG, LegalOperations, LegalTypes, Built)) {
     for (SDNode *N : Built)
       AddToWorklist(N);
     return S;
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 7fa83a5999dfe..62b092bcb1d70 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -6285,6 +6285,7 @@ SDValue TargetLowering::buildSDIVPow2WithCMov(
 /// Ref: "Hacker's Delight" or "The PowerPC Compiler Writer's Guide".
 SDValue TargetLowering::BuildSDIV(SDNode *N, SelectionDAG &DAG,
                                   bool IsAfterLegalization,
+                                  bool IsAfterLegalTypes,
                                   SmallVectorImpl<SDNode *> &Created) const {
   SDLoc dl(N);
   EVT VT = N->getValueType(0);
@@ -6405,7 +6406,12 @@ SDValue TargetLowering::BuildSDIV(SDNode *N, SelectionDAG &DAG,
     if (VT.isVector())
       WideVT = EVT::getVectorVT(*DAG.getContext(), WideVT,
                                 VT.getVectorElementCount());
-    if (isOperationLegalOrCustom(ISD::MUL, WideVT)) {
+    // Some targets like AMDGPU try to go from SDIV to SDIVREM which is then
+    // custom lowered. This is very expensive so avoid it at all costs for
+    // constant divisors.
+    if ((!IsAfterLegalTypes && isOperationExpand(ISD::SDIV, VT) &&
+         isOperationCustom(ISD::SDIVREM, VT.getScalarType())) ||
+        isOperationLegalOrCustom(ISD::MUL, WideVT)) {
       X = DAG.getNode(ISD::SIGN_EXTEND, dl, WideVT, X);
       Y = DAG.getNode(ISD::SIGN_EXTEND, dl, WideVT, Y);
       Y = DAG.getNode(ISD::MUL, dl, WideVT, X, Y);
@@ -6447,6 +6453,7 @@ SDValue TargetLowering::BuildSDIV(SDNode *N, SelectionDAG &DAG,
 /// Ref: "Hacker's Delight" or "The PowerPC Compiler Writer's Guide".
 SDValue TargetLowering::BuildUDIV(SDNode *N, SelectionDAG &DAG,
                                   bool IsAfterLegalization,
+                                  bool IsAfterLegalTypes,
                                   SmallVectorImpl<SDNode *> &Created) const {
   SDLoc dl(N);
   EVT VT = N->getValueType(0);
@@ -6588,7 +6595,12 @@ SDValue TargetLowering::BuildUDIV(SDNode *N, SelectionDAG &DAG,
     if (VT.isVector())
       WideVT = EVT::getVectorVT(*DAG.getContext(), WideVT,
                                 VT.getVectorElementCount());
-    if (isOperationLegalOrCustom(ISD::MUL, WideVT)) {
+    // Some targets like AMDGPU try to go from UDIV to UDIVREM which is then
+    // custom lowered. This is very expensive so avoid it at all costs for
+    // constant divisors.
+    if ((!IsAfterLegalTypes && isOperationExpand(ISD::UDIV, VT) &&
+         isOperationCustom(ISD::UDIVREM, VT.getScalarType())) ||
+        isOperationLegalOrCustom(ISD::MUL, WideVT)) {
       X = DAG.getNode(ISD::ZERO_EXTEND, dl, WideVT, X);
       Y = DAG.getNode(ISD::ZERO_EXTEND, dl, WideVT, Y);
       Y = DAG.getNode(ISD::MUL, dl, WideVT, X, Y);
diff --git a/llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll b/llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll
index 96e92bb3dce0d..e4756ad3817c2 100644
--- a/llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll
+++ b/llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll
@@ -7066,202 +7066,57 @@ define amdgpu_kernel void @udiv_i64_oddk_denom(ptr addrspace(1) %out, i64 %x) {
 ;
 ; GFX6-LABEL: udiv_i64_oddk_denom:
 ; GFX6:       ; %bb.0:
-; GFX6-NEXT:    s_add_u32 s4, 3, 0
-; GFX6-NEXT:    v_mov_b32_e32 v0, 0xe3e0f6
-; GFX6-NEXT:    v_add_i32_e32 v0, vcc, s4, v0
-; GFX6-NEXT:    s_addc_u32 s5, 0, 0
-; GFX6-NEXT:    s_or_b32 s4, vcc_lo, vcc_hi
-; GFX6-NEXT:    s_cmp_lg_u32 s4, 0
-; GFX6-NEXT:    s_mov_b32 s4, 0x68958c89
-; GFX6-NEXT:    s_movk_i32 s6, 0xfee0
-; GFX6-NEXT:    v_mul_lo_u32 v1, v0, s6
-; GFX6-NEXT:    v_mul_hi_u32 v2, v0, s4
-; GFX6-NEXT:    s_addc_u32 s5, s5, 0
-; GFX6-NEXT:    s_mul_i32 s6, s5, 0x68958c89
 ; GFX6-NEXT:    s_load_dwordx4 s[0:3], s[2:3], 0x9
-; GFX6-NEXT:    v_add_i32_e32 v1, vcc, v1, v2
-; GFX6-NEXT:    v_mul_lo_u32 v2, v0, s4
-; GFX6-NEXT:    v_add_i32_e32 v1, vcc, s6, v1
-; GFX6-NEXT:    v_mul_lo_u32 v3, v0, v1
-; GFX6-NEXT:    v_mul_hi_u32 v4, v0, v2
-; GFX6-NEXT:    v_mul_hi_u32 v5, v0, v1
-; GFX6-NEXT:    v_mul_hi_u32 v6, s5, v1
-; GFX6-NEXT:    v_mul_lo_u32 v1, s5, v1
-; GFX6-NEXT:    v_add_i32_e32 v3, vcc, v4, v3
-; GFX6-NEXT:    v_addc_u32_e32 v4, vcc, 0, v5, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v5, s5, v2
-; GFX6-NEXT:    v_mul_hi_u32 v2, s5, v2
-; GFX6-NEXT:    s_movk_i32 s8, 0x11f
-; GFX6-NEXT:    s_mov_b32 s9, 0x976a7377
-; GFX6-NEXT:    v_add_i32_e32 v3, vcc, v3, v5
-; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, v4, v2, vcc
-; GFX6-NEXT:    v_addc_u32_e32 v3, vcc, 0, v6, vcc
-; GFX6-NEXT:    v_add_i32_e32 v1, vcc, v2, v1
-; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, 0, v3, vcc
-; GFX6-NEXT:    v_mov_b32_e32 v3, s5
-; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v1
-; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, v3, v2, vcc
+; GFX6-NEXT:    v_mov_b32_e32 v1, 0x64c139ef
+; GFX6-NEXT:    v_mov_b32_e32 v0, 0x38f83e5
+; GFX6-NEXT:    s_mov_b32 s7, 0xf000
+; GFX6-NEXT:    s_mov_b32 s6, -1
 ; GFX6-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX6-NEXT:    v_mul_lo_u32 v2, s2, v1
-; GFX6-NEXT:    v_mul_hi_u32 v3, s2, v0
 ; GFX6-NEXT:    v_mul_hi_u32 v4, s2, v1
-; GFX6-NEXT:    v_mul_hi_u32 v5, s3, v1
-; GFX6-NEXT:    v_mul_lo_u32 v1, s3, v1
+; GFX6-NEXT:    v_mul_hi_u32 v3, s3, v1
+; GFX6-NEXT:    s_mov_b32 s5, s1
+; GFX6-NEXT:    v_mul_hi_u32 v2, s2, v0
+; GFX6-NEXT:    s_mul_i32 s1, s3, 0x64c139ef
+; GFX6-NEXT:    v_add_i32_e32 v4, vcc, s1, v4
+; GFX6-NEXT:    s_mov_b32 s4, s0
+; GFX6-NEXT:    s_mul_i32 s0, s2, 0x38f83e5
+; GFX6-NEXT:    v_addc_u32_e32 v3, vcc, 0, v3, vcc
+; GFX6-NEXT:    v_add_i32_e32 v4, vcc, s0, v4
+; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, 0, v2, vcc
 ; GFX6-NEXT:    v_add_i32_e32 v2, vcc, v3, v2
-; GFX6-NEXT:    v_addc_u32_e32 v3, vcc, 0, v4, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v4, s3, v0
 ; GFX6-NEXT:    v_mul_hi_u32 v0, s3, v0
-; GFX6-NEXT:    s_mov_b32 s4, s0
-; GFX6-NEXT:    s_mov_b32 s5, s1
-; GFX6-NEXT:    v_add_i32_e32 v2, vcc, v2, v4
-; GFX6-NEXT:    v_addc_u32_e32 v0, vcc, v3, v0, vcc
-; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, 0, v5, vcc
-; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v1
-; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, 0, v2, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v2, v0, s8
-; GFX6-NEXT:    v_mul_hi_u32 v3, v0, s9
-; GFX6-NEXT:    v_mul_lo_u32 v4, v1, s9
-; GFX6-NEXT:    v_mov_b32_e32 v5, 0x11f
-; GFX6-NEXT:    s_mov_b32 s7, 0xf000
-; GFX6-NEXT:    v_add_i32_e32 v2, vcc, v2, v3
-; GFX6-NEXT:    v_mul_lo_u32 v3, v0, s9
-; GFX6-NEXT:    v_add_i32_e32 v2, vcc, v4, v2
-; GFX6-NEXT:    v_sub_i32_e32 v4, vcc, s3, v2
-; GFX6-NEXT:    v_sub_i32_e32 v3, vcc, s2, v3
-; GFX6-NEXT:    v_subb_u32_e64 v4, s[0:1], v4, v5, vcc
-; GFX6-NEXT:    v_subrev_i32_e64 v5, s[0:1], s9, v3
-; GFX6-NEXT:    v_subbrev_u32_e64 v4, s[0:1], 0, v4, s[0:1]
-; GFX6-NEXT:    s_movk_i32 s2, 0x11e
-; GFX6-NEXT:    v_cmp_lt_u32_e64 s[0:1], s2, v4
-; GFX6-NEXT:    s_mov_b32 s9, 0x976a7376
-; GFX6-NEXT:    v_cndmask_b32_e64 v6, 0, -1, s[0:1]
-; GFX6-NEXT:    v_cmp_lt_u32_e64 s[0:1], s9, v5
-; GFX6-NEXT:    v_cndmask_b32_e64 v5, 0, -1, s[0:1]
-; GFX6-NEXT:    v_cmp_eq_u32_e64 s[0:1], s8, v4
-; GFX6-NEXT:    v_cndmask_b32_e64 v4, v6, v5, s[0:1]
-; GFX6-NEXT:    v_add_i32_e64 v5, s[0:1], 1, v0
-; GFX6-NEXT:    v_addc_u32_e64 v6, s[0:1], 0, v1, s[0:1]
-; GFX6-NEXT:    v_add_i32_e64 v7, s[0:1], 2, v0
-; GFX6-NEXT:    v_addc_u32_e64 v8, s[0:1], 0, v1, s[0:1]
-; GFX6-NEXT:    v_cmp_ne_u32_e64 s[0:1], 0, v4
-; GFX6-NEXT:    v_cndmask_b32_e64 v4, v5, v7, s[0:1]
-; GFX6-NEXT:    v_cndmask_b32_e64 v5, v6, v8, s[0:1]
-; GFX6-NEXT:    v_mov_b32_e32 v6, s3
-; GFX6-NEXT:    v_subb_u32_e32 v2, vcc, v6, v2, vcc
-; GFX6-NEXT:    v_cmp_lt_u32_e32 vcc, s2, v2
-; GFX6-NEXT:    v_cndmask_b32_e64 v6, 0, -1, vcc
-; GFX6-NEXT:    v_cmp_lt_u32_e32 vcc, s9, v3
-; GFX6-NEXT:    v_cndmask_b32_e64 v3, 0, -1, vcc
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, s8, v2
-; GFX6-NEXT:    v_cndmask_b32_e32 v2, v6, v3, vcc
-; GFX6-NEXT:    v_cmp_ne_u32_e32 vcc, 0, v2
-; GFX6-NEXT:    s_mov_b32 s6, -1
-; GFX6-NEXT:    v_cndmask_b32_e32 v1, v1, v5, vcc
-; GFX6-NEXT:    v_cndmask_b32_e32 v0, v0, v4, vcc
+; GFX6-NEXT:    v_addc_u32_e64 v3, s[0:1], 0, 0, vcc
+; GFX6-NEXT:    s_mul_i32 s0, s3, 0x38f83e5
+; GFX6-NEXT:    v_add_i32_e32 v2, vcc, s0, v2
+; GFX6-NEXT:    v_addc_u32_e32 v0, vcc, v0, v3, vcc
+; GFX6-NEXT:    v_mov_b32_e32 v1, 0
+; GFX6-NEXT:    v_lshrrev_b32_e32 v0, 2, v0
 ; GFX6-NEXT:    buffer_store_dwordx2 v[0:1], off, s[4:7], 0
 ; GFX6-NEXT:    s_endpgm
 ;
 ; GFX9-LABEL: udiv_i64_oddk_denom:
 ; GFX9:       ; %bb.0:
-; GFX9-NEXT:    s_add_u32 s0, 3, 0
-; GFX9-NEXT:    v_mov_b32_e32 v0, 0xe3e0f6
-; GFX9-NEXT:    s_addc_u32 s1, 0, 0
-; GFX9-NEXT:    v_add_co_u32_e32 v0, vcc, s0, v0
 ; GFX9-NEXT:    s_load_dwordx4 s[4:7], s[2:3], 0x24
-; GFX9-NEXT:    s_cmp_lg_u64 vcc, 0
-; GFX9-NEXT:    v_readfirstlane_b32 s2, v0
-; GFX9-NEXT:    s_addc_u32 s0, s1, 0
-; GFX9-NEXT:    s_mul_i32 s3, s2, 0xfffffee0
-; GFX9-NEXT:    s_mul_hi_u32 s8, s2, 0x68958c89
-; GFX9-NEXT:    s_mul_i32 s1, s0, 0x68958c89
-; GFX9-NEXT:    s_add_i32 s3, s8, s3
-; GFX9-NEXT:    s_add_i32 s3, s3, s1
-; GFX9-NEXT:    s_mul_i32 s9, s2, 0x68958c89
-; GFX9-NEXT:    s_mul_hi_u32 s1, s2, s3
-; GFX9-NEXT:    s_mul_i32 s8, s2, s3
-; GFX9-NEXT:    s_mul_hi_u32 s2, s2, s9
-; GFX9-NEXT:    s_add_u32 s2, s2, s8
-; GFX9-NEXT:    s_addc_u32 s1, 0, s1
-; GFX9-NEXT:    s_mul_hi_u32 s10, s0, s9
-; GFX9-NEXT:    s_mul_i32 s9, s0, s9
-; GFX9-NEXT:    s_add_u32 s2, s2, s9
-; GFX9-NEXT:    s_mul_hi_u32 s8, s0, s3
-; GFX9-NEXT:    s_addc_u32 s1, s1, s10
-; GFX9-NEXT:    s_addc_u32 s2, s8, 0
-; GFX9-NEXT:    s_mul_i32 s3, s0, s3
-; GFX9-NEXT:    s_add_u32 s1, s1, s3
-; GFX9-NEXT:    s_addc_u32 s2, 0, s2
-; GFX9-NEXT:    v_add_co_u32_e32 v0, vcc, s1, v0
-; GFX9-NEXT:    s_cmp_lg_u64 vcc, 0
-; GFX9-NEXT:    s_addc_u32 s0, s0, s2
-; GFX9-NEXT:    v_readfirstlane_b32 s3, v0
+; GFX9-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX9-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX9-NEXT:    s_mul_i32 s2, s6, s0
-; GFX9-NEXT:    s_mul_hi_u32 s8, s6, s3
-; GFX9-NEXT:    s_mul_hi_u32 s1, s6, s0
-; GFX9-NEXT:    s_add_u32 s2, s8, s2
-; GFX9-NEXT:    s_addc_u32 s1, 0, s1
-; GFX9-NEXT:    s_mul_hi_u32 s9, s7, s3
-; GFX9-NEXT:    s_mul_i32 s3, s7, s3
-; GFX9-NEXT:    s_add_u32 s2, s2, s3
-; GFX9-NEXT:    s_mul_hi_u32 s8, s7, s0
-; GFX9-NEXT:    s_addc_u32 s1, s1, s9
-; GFX9-NEXT:    s_addc_u32 s2, s8, 0
-; GFX9-NEXT:    s_mul_i32 s0, s7, s0
-; GFX9-NEXT:    s_add_u32 s3, s1, s0
-; GFX9-NEXT:    s_addc_u32 s2, 0, s2
-; GFX9-NEXT:    s_mul_i32 s0, s3, 0x11f
-; GFX9-NEXT:    s_mul_hi_u32 s8, s3, 0x976a7377
-; GFX9-NEXT:    s_add_i32 s0, s8, s0
-; GFX9-NEXT:    s_mul_i32 s8, s2, 0x976a7377
-; GFX9-NEXT:    s_mul_i32 s9, s3, 0x976a7377
-; GFX9-NEXT:    s_add_i32 s8, s0, s8
-; GFX9-NEXT:    v_mov_b32_e32 v0, s9
-; GFX9-NEXT:    s_sub_i32 s0, s7, s8
-; GFX9-NEXT:    v_sub_co_u32_e32 v0, vcc, s6, v0
-; GFX9-NEXT:    s_mov_b32 s1, 0x976a7377
-; GFX9-NEXT:    s_cmp_lg_u64 vcc, 0
-; GFX9-NEXT:    s_subb_u32 s6, s0, 0x11f
-; GFX9-NEXT:    v_subrev_co_u32_e64 v1, s[0:1], s1, v0
-; GFX9-NEXT:    s_cmp_lg_u64 s[0:1], 0
-; GFX9-NEXT:    s_subb_u32 s6, s6, 0
-; GFX9-NEXT:    s_cmpk_gt_u32 s6, 0x11e
-; GFX9-NEXT:    s_mov_b32 s10, 0x976a7376
-; GFX9-NEXT:    s_cselect_b32 s9, -1, 0
-; GFX9-NEXT:    v_cmp_lt_u32_e64 s[0:1], s10, v1
-; GFX9-NEXT:    s_cmpk_eq_i32 s6, 0x11f
-; GFX9-NEXT:    v_cndmask_b32_e64 v1, 0, -1, s[0:1]
-; GFX9-NEXT:    v_mov_b32_e32 v3, s9
-; GFX9-NEXT:    s_cselect_b64 s[0:1], -1, 0
-; GFX9-NEXT:    v_cndmask_b32_e64 v1, v3, v1, s[0:1]
-; GFX9-NEXT:    s_add_u32 s0, s3, 1
-; GFX9-NEXT:    s_addc_u32 s6, s2, 0
-; GFX9-NEXT:    s_add_u32 s1, s3, 2
-; GFX9-NEXT:    s_addc_u32 s9, s2, 0
-; GFX9-NEXT:    v_mov_b32_e32 v3, s0
-; GFX9-NEXT:    v_mov_b32_e32 v4, s1
-; GFX9-NEXT:    v_cmp_ne_u32_e64 s[0:1], 0, v1
-; GFX9-NEXT:    v_cndmask_b32_e64 v3, v3, v4, s[0:1]
-; GFX9-NEXT:    v_mov_b32_e32 v1, s6
-; GFX9-NEXT:    v_mov_b32_e32 v4, s9
-; GFX9-NEXT:    s_cmp_lg_u64 vcc, 0
-; GFX9-NEXT:    v_cndmask_b32_e64 v1, v1, v4, s[0:1]
-; GFX9-NEXT:    s_subb_u32 s0, s7, s8
-; GFX9-NEXT:    s_cmpk_gt_u32 s0, 0x11e
-; GFX9-NEXT:    s_cselect_b32 s1, -1, 0
-; GFX9-NEXT:    v_cmp_lt_u32_e32 vcc, s10, v0
-; GFX9-NEXT:    s_cmpk_eq_i32 s0, 0x11f
-; GFX9-NEXT:    v_cndmask_b32_e64 v0, 0, -1, vcc
-; GFX9-NEXT:    v_mov_b32_e32 v4, s1
-; GFX9-NEXT:    s_cselect_b64 vcc, -1, 0
-; GFX9-NEXT:    v_cndmask_b32_e32 v0, v4, v0, vcc
-; GFX9-NEXT:    v_mov_b32_e32 v4, s2
-; GFX9-NEXT:    v_cmp_ne_u32_e32 vcc, 0, v0
-; GFX9-NEXT:    v_mov_b32_e32 v0, s3
-; GFX9-NEXT:    v_mov_b32_e32 v2, 0
-; GFX9-NEXT:    v_cndmask_b32_e32 v1, v4, v1, vcc
-; GFX9-NEXT:    v_cndmask_b32_e32 v0, v0, v3, vcc
-; GFX9-NEXT:    global_store_dwordx2 v2, v[0:1], s[4:5]
+; GFX9-NEXT:    s_mul_hi_u32 s0, s6, 0x38f83e5
+; GFX9-NEXT:    s_mul_i32 s1, s6, 0x38f83e5
+; GFX9-NEXT:    s_mul_i32 s3, s7, 0x64c139ef
+; GFX9-NEXT:    s_mul_hi_u32 s6, s6, 0x64c139ef
+; GFX9-NEXT:    s_mul_hi_u32 s2, s7, 0x64c139ef
+; GFX9-NEXT:    s_add_u32 s3, s3, s6
+; GFX9-NEXT:    s_addc_u32 s2, s2, 0
+; GFX9-NEXT:    s_add_u32 s1, s1, s3
+; GFX9-NEXT:    s_addc_u32 s0, s0, 0
+; GFX9-NEXT:    s_add_u32 s0, s2, s0
+; GFX9-NEXT:    s_addc_u32 s1, 0, 0
+; GFX9-NEXT:    s_mul_i32 s3, s7, 0x38f83e5
+; GFX9-NEXT:    s_mul_hi_u32 s2, s7, 0x38f83e5
+; GFX9-NEXT:    s_add_u32 s0, s3, s0
+; GFX9-NEXT:    s_addc_u32 s0, s2, s1
+; GFX9-NEXT:    s_lshr_b32 s0, s0, 2
+; GFX9-NEXT:    v_mov_b32_e32 v0, s0
+; GFX9-NEXT:    global_store_dwordx2 v1, v[0:1], s[4:5]
 ; GFX9-NEXT:    s_endpgm
   %r = udiv i64 %x, 1235195949943
   store i64 %r, ptr addrspace(1) %out
@@ -7405,84 +7260,34 @@ define amdgpu_kernel void @udiv_v2i64_mixed_pow2k_denom(ptr addrspace(1) %out, <
 ; GFX6:       ; %bb.0:
 ; GFX6-NEXT:    s_load_dwordx4 s[4:7], s[2:3], 0xd
 ; GFX6-NEXT:    s_load_dwordx2 s[0:1], s[2:3], 0x9
-; GFX6-NEXT:    s_mov_b32 s2, 0x2ff2fc01
-; GFX6-NEXT:    v_bfrev_b32_e32 v0, 7
-; GFX6-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX6-NEXT:    s_lshr_b64 s[4:5], s[4:5], 12
-; GFX6-NEXT:    s_add_u32 s2, 0xe037f, s2
-; GFX6-NEXT:    v_add_i32_e32 v0, vcc, s2, v0
-; GFX6-NEXT:    s_addc_u32 s3, 0, 0
-; GFX6-NEXT:    s_or_b32 s2, vcc_lo, vcc_hi
-; GFX6-NEXT:    s_cmp_lg_u32 s2, 0
-; GFX6-NEXT:    s_movk_i32 s2, 0xf001
-; GFX6-NEXT:    v_mul_hi_u32 v1, v0, s2
-; GFX6-NEXT:    v_mul_lo_u32 v2, v0, s2
-; GFX6-NEXT:    s_addc_u32 s8, s3, 0x1000ff
-; GFX6-NEXT:    s_mul_i32 s3, s8, 0xfffff001
-; GFX6-NEXT:    v_sub_i32_e32 v1, vcc, v1, v0
-; GFX6-NEXT:    v_add_i32_e32 v1, vcc, s3, v1
-; GFX6-NEXT:    v_mul_lo_u32 v3, v0, v1
-; GFX6-NEXT:    v_mul_hi_u32 v4, v0, v2
-; GFX6-NEXT:    v_mul_hi_u32 v5, v0, v1
-; GFX6-NEXT:    v_mul_hi_u32 v6, s8, v1
-; GFX6-NEXT:    v_mul_lo_u32 v1, s8, v1
-; GFX6-NEXT:    v_add_i32_e32 v3, vcc, v4, v3
-; GFX6-NEXT:    v_addc_u32_e32 v4, vcc, 0, v5, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v5, s8, v2
-; GFX6-NEXT:    v_mul_hi_u32 v2, s8, v2
+; GFX6-NEXT:    v_mov_b32_e32 v2, 0x10010011
+; GFX6-NEXT:    v_mov_b32_e32 v0, 0x100100
 ; GFX6-NEXT:    s_mov_b32 s3, 0xf000
-; GFX6-NEXT:    s_mov_b32 s2, -1
-; GFX6-NEXT:    v_add_i32_e32 v3, vcc, v3, v5
-; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, v4, v2, vcc
-; GFX6-NEXT:    v_addc_u32_e32 v3, vcc, 0, v6, vcc
+; GFX6-NEXT:    s_waitcnt lgkmcnt(0)
+; GFX6-NEXT:    v_mul_hi_u32 v3, s6, v2
+; GFX6-NEXT:    v_mul_hi_u32 v2, s7, v2
+; GFX6-NEXT:    v_mul_hi_u32 v1, s6, v0
+; GFX6-NEXT:    s_mul_i32 s9, s7, 0x10010011
+; GFX6-NEXT:    v_add_i32_e32 v3, vcc, s9, v3
+; GFX6-NEXT:    s_mul_i32 s8, s6, 0x100100
+; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, 0, v2, vcc
+; GFX6-NEXT:    v_add_i32_e32 v3, vcc, s8, v3
+; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, 0, v1, vcc
 ; GFX6-NEXT:    v_add_i32_e32 v1, vcc, v2, v1
-; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, 0, v3, vcc
-; GFX6-NEXT:    v_mov_b32_e32 v3, s8
-; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v1
-; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, v3, v2, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v2, s6, v1
-; GFX6-NEXT:    v_mul_hi_u32 v3, s6, v0
-; GFX6-NEXT:    v_mul_hi_u32 v4, s6, v1
-; GFX6-NEXT:    v_mul_hi_u32 v5, s7, v1
-; GFX6-NEXT:    v_mul_lo_u32 v1, s7, v1
-; GFX6-NEXT:    v_add_i32_e32 v2, vcc, v3, v2
-; GFX6-NEXT:    v_addc_u32_e32 v3, vcc, 0, v4, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v4, s7, v0
 ; GFX6-NEXT:    v_mul_hi_u32 v0, s7, v0
-; GFX6-NEXT:    s_movk_i32 s8, 0xfff
-; GFX6-NEXT:    v_add_i32_e32 v2, vcc, v2, v4
-; GFX6-NEXT:    v_addc_u32_e32 v0, vcc, v3, v0, vcc
-; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, 0, v5, vcc
-; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v1
-; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, 0, v2, vcc
-; GFX6-NEXT:    v_mul_lo_u32 v4, v1, s8
-; GFX6-NEXT:    v_mul_hi_u32 v5, v0, s8
-; GFX6-NEXT:    v_add_i32_e32 v2, vcc, 1, v0
-; GFX6-NEXT:    v_mul_lo_u32 v8, v0, s8
-; GFX6-NEXT:    v_addc_u32_e32 v3, vcc, 0, v1, vcc
-; GFX6-NEXT:    v_add_i32_e32 v6, vcc, 2, v0
-; GFX6-NEXT:    v_addc_u32_e32 v7, vcc, 0, v1, vcc
-; GFX6-NEXT:    v_add_i32_e32 v4, vcc, v4, v5
-; GFX6-NEXT:    v_mov_b32_e32 v5, s7
-; GFX6-NEXT:    v_sub_i32_e32 v8, vcc, s6, v8
-; GFX6-NEXT:    v_subb_u32_e32 v4, vcc, v5, v4, vcc
-; GFX6-NEXT:    v_subrev_i32_e32 v5, vcc, s8, v8
-; GFX6-NEXT:    v_subbrev_u32_e32 v9, vcc, 0, v4, vcc
-; GFX6-NEXT:    s_movk_i32 s6, 0xffe
-; GFX6-NEXT:    v_cmp_lt_u32_e32 vcc, s6, v5
-; GFX6-NEXT:    v_cndmask_b32_e64 v5, 0, -1, vcc
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, 0, v9
-; GFX6-NEXT:    v_cndmask_b32_e32 v5, -1, v5, vcc
-; GFX6-NEXT:    v_cmp_ne_u32_e32 vcc, 0, v5
-; GFX6-NEXT:    v_cndmask_b32_e32 v2, v2, v6, vcc
-; GFX6-NEXT:    v_cndmask_b32_e32 v3, v3, v7, vcc
-; GFX6-NEXT:    v_cmp_lt_u32_e32 vcc, s6, v8
-; GFX6-NEXT:    v_cndmask_b32_e64 v5, 0, -1, vcc
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, 0, v4
-; GFX6-NEXT:    v_cndmask_b32_e32 v4, -1, v5, vcc
-; GFX6-NEXT:    v_cmp_ne_u32_e32 vcc, 0, v4
-; GFX6-NEXT:    v_cndmask_b32_e32 v3, v1, v3, vcc
-; GFX6-NEXT:    v_cndmask_b32_e32 v2, v0, v2, vcc
+; GFX6-NEXT:    v_addc_u32_e64 v2, s[8:9], 0, 0, vcc
+; GFX6-NEXT:    s_mul_i32 s8, s7, 0x100100
+; GFX6-NEXT:    v_add_i32_e32 v3, vcc, s8, v1
+; GFX6-NEXT:    v_addc_u32_e32 v2, vcc, v0, v2, vcc
+; GFX6-NEXT:    v_mov_b32_e32 v1, s7
+; GFX6-NEXT:    v_sub_i32_e32 v0, vcc, s6, v3
+; GFX6-NEXT:    v_subb_u32_e32 v1, vcc, v1, v2, vcc
+; GFX6-NEXT:    v_lshr_b64 v[0:1], v[0:1], 1
+; GFX6-NEXT:    s_lshr_b64 s[4:5], s[4:5], 12
+; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v3
+; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, v1, v2, vcc
+; GFX6-NEXT:    v_lshr_b64 v[2:3], v[0:1], 11
+; GFX6-NEXT:    s_mov_b32 s2, -1
 ; GFX6-NEXT:    v_mov_b32_e32 v0, s4
 ; GFX6-NEXT:    v_mov_b32_e32 v1, s5
 ; GFX6-NEXT:    buffer_store_dwordx4 v[0:3], off, s[0:3], 0
@@ -7492,95 +7297,34 @@ define amdgpu_kernel void @udiv_v2i64_mixed_pow2k_denom(ptr addrspa...
[truncated]

arsenm
arsenm approved these changes Aug 7, 2024
View reviewed changes
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Where is the AMDGPU regression? can it be fixed?

MaskRay
MaskRay approved these changes Aug 7, 2024
View reviewed changes
Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

LGTM.
The internal Halide ARM target still passes with this patch.

@davemgreen might want to check the naming of the test llvm/test/CodeGen/ARM/div-by-constant-to-mul-crash.ll
I think it's too specific to use a "*-crash" to name the test.
One of the existing *div*.ll tests might be better.

davemgreen
davemgreen reviewed Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

The ARM test LGTM which a minor suggestion to update the triple used. Thanks.

llvm/test/CodeGen/ARM/div-by-constant-to-mul-crash.ll Outdated
@@ -0,0 +1,56 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
; RUN: llc < %s -mtriple=arm--linux-gnueabihf -mcpu= -mattr=+neon | FileCheck %s
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what -mcpu= does, but can it be removed? And can you use armv7--linux-gnueabihf if it still shows the same issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Pierre-vh
Reapply "[AMDGPU] Always lower s/udiv64 by constant to MUL"
4903632 
Reland llvm#100723, fixing the ARM issue at the cost of a small regression in AMDGPU.

Solves llvm#100383
@Pierre-vh
Copy link
Contributor Author

Where is the AMDGPU regression? can it be fixed?

Some small changes in fshr.ll (current version is on the left)
image

@Pierre-vh Pierre-vh merged commit 7389545 into llvm:main Aug 12, 2024
5 of 8 checks passed
@Pierre-vh Pierre-vh deleted the reapply-div-by-k branch August 12, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davemgreen davemgreen davemgreen left review comments

@arsenm arsenm arsenm approved these changes

@MaskRay MaskRay MaskRay approved these changes

Assignees
No one assigned
Labels
backend:AMDGPU backend:ARM llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Pierre-vh @llvmbot @arsenm @MaskRay @davemgreen