[llvm] [X86] Fix reversed condition in ADC/SBB fold of COND_A compares (PR #228693)

[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>

#<!-- -->161388 added a COND_A case to combineAddOrSubToADCOrSBB that reuses the flags of `CMP X, Y` for ADC/SBB without swapping the operands, computing X +/- (X <u Y) instead of X +/- (X >u Y).

It was unreachable until #<!-- -->221290 started emitting X86ISD::CMP instead of X86ISD::SUB for compares of atomic loads.

Remove it. The `(a - b) - (a > b)` case #<!-- -->161388 targeted goes through the X86ISD::SUB path and is unaffected.

Fixes #<!-- -->228457

Assisted-by: Claude Opus 5.5

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


2 Files Affected:

- (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (-10) 
- (modified) llvm/test/CodeGen/X86/atomic-cmp-fold.ll (+53) 


``````````diff
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 7817c86155e5b..971186abd7339 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -53931,16 +53931,6 @@ 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 &&
         EFLAGS.getValueType().isInteger() &&
         !isa<ConstantSDNode>(EFLAGS.getOperand(1))) {
diff --git a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
index c3c1af48422b7..2cd181673517a 100644
--- a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
+++ b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
@@ -368,3 +368,56 @@ define i1 @cmp32_volatile(ptr %p) {
   %c = icmp eq i32 %v, 0
   ret i1 %c
 }
+
+; The carry flag of a folded "cmp a, (p)" means a <u *p, so it cannot be used
+; as-is to add or subtract "a >u *p" (#228457).
+define i32 @add_ugt(i32 %a, ptr %p) {
+; X64-LABEL: add_ugt:
+; X64:       # %bb.0:
+; X64-NEXT:    xorl %eax, %eax
+; X64-NEXT:    cmpl (%rsi), %edi
+; X64-NEXT:    seta %al
+; X64-NEXT:    addl %edi, %eax
+; X64-NEXT:    retq
+;
+; X86-LABEL: add_ugt:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
+; X86-NEXT:    xorl %eax, %eax
+; X86-NEXT:    cmpl (%ecx), %edx
+; X86-NEXT:    seta %al
+; X86-NEXT:    addl %edx, %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:    xorl %ecx, %ecx
+; X64-NEXT:    cmpl (%rsi), %edi
+; X64-NEXT:    seta %cl
+; X64-NEXT:    subl %ecx, %eax
+; X64-NEXT:    retq
+;
+; X86-LABEL: sub_ugt:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    xorl %edx, %edx
+; X86-NEXT:    cmpl (%ecx), %eax
+; X86-NEXT:    seta %dl
+; X86-NEXT:    subl %edx, %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
+}

``````````

</details>


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