Skip to content

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Aug 31, 2025

select(ugt x, y), sub(x, y), sub(0, sub(x, y)) -> abdu(x, y)

This is because -diff is the same as y - x.

@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Aug 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2025

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: AZero13 (AZero13)

Changes

select(ugt x, y), sub(x, y), sub(0, sub(x, y)) -> abdu(x, y)

This is because -diff is the same as y - x.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+23-16)
  • (modified) llvm/test/CodeGen/X86/abdu.ll (+27)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 55aff0460e7d7..f6dc79db75c53 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -12177,27 +12177,34 @@ SDValue DAGCombiner::foldSelectToABD(SDValue LHS, SDValue RHS, SDValue True,
   case ISD::SETGT:
   case ISD::SETGE:
   case ISD::SETUGT:
-  case ISD::SETUGE:
-    if (sd_match(True, m_Sub(m_Specific(LHS), m_Specific(RHS))) &&
-        sd_match(False, m_Sub(m_Specific(RHS), m_Specific(LHS))))
-      return DAG.getNode(ABDOpc, DL, VT, LHS, RHS);
-    if (sd_match(True, m_Sub(m_Specific(RHS), m_Specific(LHS))) &&
-        sd_match(False, m_Sub(m_Specific(LHS), m_Specific(RHS))) &&
-        hasOperation(ABDOpc, VT))
-      return DAG.getNegative(DAG.getNode(ABDOpc, DL, VT, LHS, RHS), DL, VT);
+  case ISD::SETUGE: {
+    if (sd_match(True, m_Sub(m_Specific(LHS), m_Specific(RHS)))) {
+      if (sd_match(False, m_Sub(m_Specific(RHS), m_Specific(LHS))))
+        return DAG.getNode(ABDOpc, DL, VT, LHS, RHS);
+    }
+
+    if (sd_match(True, m_Sub(m_Specific(RHS), m_Specific(LHS)))) {
+      if (sd_match(False, m_Sub(m_Specific(LHS), m_Specific(RHS))))
+        return DAG.getNegative(DAG.getNode(ABDOpc, DL, VT, LHS, RHS), DL, VT);
+    }
     break;
+  }
   case ISD::SETLT:
   case ISD::SETLE:
   case ISD::SETULT:
-  case ISD::SETULE:
-    if (sd_match(True, m_Sub(m_Specific(RHS), m_Specific(LHS))) &&
-        sd_match(False, m_Sub(m_Specific(LHS), m_Specific(RHS))))
-      return DAG.getNode(ABDOpc, DL, VT, LHS, RHS);
-    if (sd_match(True, m_Sub(m_Specific(LHS), m_Specific(RHS))) &&
-        sd_match(False, m_Sub(m_Specific(RHS), m_Specific(LHS))) &&
-        hasOperation(ABDOpc, VT))
-      return DAG.getNegative(DAG.getNode(ABDOpc, DL, VT, LHS, RHS), DL, VT);
+  case ISD::SETULE: {
+    if (sd_match(True, m_Sub(m_Specific(RHS), m_Specific(LHS)))) {
+      if (sd_match(False, m_Sub(m_Specific(LHS), m_Specific(RHS))))
+        return DAG.getNode(ABDOpc, DL, VT, LHS, RHS);
+    }
+
+    if (sd_match(True, m_Sub(m_Specific(LHS), m_Specific(RHS)))) {
+      if (sd_match(False, m_Sub(m_Specific(RHS), m_Specific(LHS))) &&
+          hasOperation(ABDOpc, VT))
+        return DAG.getNegative(DAG.getNode(ABDOpc, DL, VT, LHS, RHS), DL, VT);
+    }
     break;
+  }
   default:
     break;
   }
diff --git a/llvm/test/CodeGen/X86/abdu.ll b/llvm/test/CodeGen/X86/abdu.ll
index 043c9155f52f9..b9e01fda29615 100644
--- a/llvm/test/CodeGen/X86/abdu.ll
+++ b/llvm/test/CodeGen/X86/abdu.ll
@@ -953,6 +953,33 @@ define i128 @abd_select_i128(i128 %a, i128 %b) nounwind {
   ret i128 %sub
 }
 
+define i32 @abdu_select(i32 %x, i32 %y) {
+; X86-LABEL: abdu_select:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl %eax, %edx
+; X86-NEXT:    subl %ecx, %edx
+; X86-NEXT:    negl %edx
+; X86-NEXT:    subl %ecx, %eax
+; X86-NEXT:    cmovbel %edx, %eax
+; X86-NEXT:    retl
+;
+; X64-LABEL: abdu_select:
+; X64:       # %bb.0:
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    subl %esi, %eax
+; X64-NEXT:    negl %eax
+; X64-NEXT:    subl %esi, %edi
+; X64-NEXT:    cmoval %edi, %eax
+; X64-NEXT:    retq
+  %sub = sub i32 %x, %y
+  %cmp = icmp ugt i32 %x, %y
+  %sub1 = sub i32 0, %sub
+  %cond = select i1 %cmp, i32 %sub, i32 %sub1
+  ret i32 %cond
+}
+
 declare i8 @llvm.abs.i8(i8, i1)
 declare i16 @llvm.abs.i16(i16, i1)
 declare i32 @llvm.abs.i32(i32, i1)

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

Failed Tests (5):
LLVM :: CodeGen/AArch64/abds-neg.ll
LLVM :: CodeGen/AArch64/abdu-neg.ll
LLVM :: CodeGen/RISCV/abds-neg.ll
LLVM :: CodeGen/RISCV/abdu-neg.ll
LLVM :: CodeGen/X86/abds-neg.ll

@RKSimon
Copy link
Collaborator

RKSimon commented Sep 1, 2025

Please can you add an alive2 link

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

How come we're not already catching this with neg(sub(x,y)) -> sub(y,x)?

@AZero13 AZero13 force-pushed the ctpops branch 2 times, most recently from 3a61565 to 2181b38 Compare September 25, 2025 01:42
; CHECK-NEXT: sbcs x11, x1, x3
; CHECK-NEXT: csel x0, x10, x8, lt
; CHECK-NEXT: csel x1, x11, x9, lt
; CHECK-NEXT: subs x8, x0, x2
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks worse?

@RKSimon
Copy link
Collaborator

RKSimon commented Sep 25, 2025

How come we're not already catching this with neg(sub(x,y)) -> sub(y,x)?

Any progress on investigating this please?

…lection

select(ugt x, y), sub(x, y), sub(0, sub(x, y)) -> abdu(x, y)

This is because -diff is the same as y - x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants