[llvm] [X86] Swap CMP operands for COND_A in ADC/SBB fold (PR #228710)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-x86

Author: Aochang Liu (HelloWorldU)

<details>
<summary>Changes</summary>

CombineAddOrSubToADCOrSBB turns X +/- setcc(COND_A, SUB A, B) into ADC/SBB X, 0 using a swapped SUB B, A, but did not handle X86ISD::CMP. Since #<!-- -->221290 a compare of an atomic load is emitted as X86ISD::CMP, so after #<!-- -->228693 `a + (a > *p)` no longer forms ADC. Handle CMP the same way by recreating the node with its own opcode; the load still folds as `cmp (p), a`.

Depends on #<!-- -->228693 .

Assisted-by: Claude Opus 5.5

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


2 Files Affected:

- (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+8-17) 
- (modified) llvm/test/CodeGen/X86/atomic-cmp-fold.ll (+91) 


``````````diff
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 7817c86155e5b..a2923e134b411 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -53931,27 +53931,18 @@ static SDValue combineAddOrSubToADCOrSBB(bool IsSub, const SDLoc &DL, EVT VT,
     // Do not flip "e > c", where "c" is a constant, because Cmp instruction
     // cannot take an immediate as its first operand.
     //
-    // If EFLAGS is from a CMP that compares the same operands as the earlier
-    // SUB producing X (i.e. CMP X, Y), we can directly use the carry flag with
-    // SBB/ADC without creating a flipped SUB.
-    if (EFLAGS.getOpcode() == X86ISD::CMP &&
-        EFLAGS.getValueType().isInteger() && X == EFLAGS.getOperand(0)) {
-      return DAG.getNode(IsSub ? X86ISD::SBB : X86ISD::ADC, DL,
-                         DAG.getVTList(VT, MVT::i32), X,
-                         DAG.getConstant(0, DL, VT), EFLAGS);
-    }
-
-    if (EFLAGS.getOpcode() == X86ISD::SUB &&
+    if ((EFLAGS.getOpcode() == X86ISD::SUB ||
+         EFLAGS.getOpcode() == X86ISD::CMP) &&
         EFLAGS.getValueType().isInteger() &&
         !isa<ConstantSDNode>(EFLAGS.getOperand(1))) {
-      // Only create NewSub if we know one of the folds will succeed to avoid
-      // introducing a temporary node that may persist and affect one-use checks
-      // below.
+      // Only create a swapped node if we know one of the folds will succeed to
+      // avoid introducing a temporary node that may persist and affect one-use
+      // checks below.
       if (EFLAGS.getNode()->hasOneUse()) {
-        SDValue NewSub = DAG.getNode(
-            X86ISD::SUB, SDLoc(EFLAGS), EFLAGS.getNode()->getVTList(),
+        SDValue Swapped = DAG.getNode(
+            EFLAGS.getOpcode(), SDLoc(EFLAGS), EFLAGS.getNode()->getVTList(),
             EFLAGS.getOperand(1), EFLAGS.getOperand(0));
-        SDValue NewEFLAGS = NewSub.getValue(EFLAGS.getResNo());
+        SDValue NewEFLAGS = Swapped.getValue(EFLAGS.getResNo());
         return DAG.getNode(IsSub ? X86ISD::SBB : X86ISD::ADC, DL,
                            DAG.getVTList(VT, MVT::i32), X,
                            DAG.getConstant(0, DL, VT), NewEFLAGS);
diff --git a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
index c3c1af48422b7..74306755b0b11 100644
--- a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
+++ b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
@@ -368,3 +368,94 @@ define i1 @cmp32_volatile(ptr %p) {
   %c = icmp eq i32 %v, 0
   ret i1 %c
 }
+
+; Adding or subtracting "a >u *p" uses the carry flag of the swapped compare
+; "cmp (p), a", not of "cmp a, (p)", which means a <u *p (#228457).
+define i32 @add_ugt(i32 %a, ptr %p) {
+; X64-LABEL: add_ugt:
+; X64:       # %bb.0:
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    cmpl %edi, (%rsi)
+; X64-NEXT:    adcl $0, %eax
+; X64-NEXT:    retq
+;
+; X86-LABEL: add_ugt:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    cmpl %eax, (%ecx)
+; X86-NEXT:    adcl $0, %eax
+; X86-NEXT:    retl
+  %v = load atomic i32, ptr %p seq_cst, align 4
+  %c = icmp ugt i32 %a, %v
+  %z = zext i1 %c to i32
+  %r = add i32 %a, %z
+  ret i32 %r
+}
+
+define i32 @sub_ugt(i32 %a, ptr %p) {
+; X64-LABEL: sub_ugt:
+; X64:       # %bb.0:
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    cmpl %edi, (%rsi)
+; X64-NEXT:    sbbl $0, %eax
+; X64-NEXT:    retq
+;
+; X86-LABEL: sub_ugt:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    cmpl %eax, (%ecx)
+; X86-NEXT:    sbbl $0, %eax
+; X86-NEXT:    retl
+  %v = load atomic i32, ptr %p seq_cst, align 4
+  %c = icmp ugt i32 %a, %v
+  %z = zext i1 %c to i32
+  %r = sub i32 %a, %z
+  ret i32 %r
+}
+
+define i32 @add_ugt_other(i32 %a, i32 %x, ptr %p) {
+; X64-LABEL: add_ugt_other:
+; X64:       # %bb.0:
+; X64-NEXT:    movl %esi, %eax
+; X64-NEXT:    cmpl %edi, (%rdx)
+; X64-NEXT:    adcl $0, %eax
+; X64-NEXT:    retq
+;
+; X86-LABEL: add_ugt_other:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl (%ecx), %ecx
+; X86-NEXT:    cmpl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    adcl $0, %eax
+; X86-NEXT:    retl
+  %v = load atomic i32, ptr %p seq_cst, align 4
+  %c = icmp ugt i32 %a, %v
+  %z = zext i1 %c to i32
+  %r = add i32 %x, %z
+  ret i32 %r
+}
+
+define i32 @add_ugt_lhs(i32 %a, ptr %p) {
+; X64-LABEL: add_ugt_lhs:
+; X64:       # %bb.0:
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    cmpl (%rsi), %edi
+; X64-NEXT:    adcl $0, %eax
+; X64-NEXT:    retq
+;
+; X86-LABEL: add_ugt_lhs:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    cmpl (%ecx), %eax
+; X86-NEXT:    adcl $0, %eax
+; X86-NEXT:    retl
+  %v = load atomic i32, ptr %p seq_cst, align 4
+  %c = icmp ugt i32 %v, %a
+  %z = zext i1 %c to i32
+  %r = add i32 %a, %z
+  ret i32 %r
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/228710