[llvm] [X86] Fix reversed condition in ADC/SBB fold of COND_A compares (PR #228693)
Aochang Liu via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 3 03:51:41 PDT 2026
https://github.com/HelloWorldU created https://github.com/llvm/llvm-project/pull/228693
#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
>From c04637bdc7a4e5f85c5a8f987e12dcaf4826093e Mon Sep 17 00:00:00 2001
From: HelloWorldU <asd001liu at gmail.com>
Date: Sat, 3 Oct 2026 18:46:46 +0800
Subject: [PATCH 1/2] [X86] Add tests for add/sub of an ugt compare of an
atomic load (NFC)
Pre-commit tests for #228457. The checks show the current miscompile: the
carry flag of `cmp a, (p)` (a <u *p) is used to add/subtract a >u *p.
Assisted-by: Claude Opus 5.5
---
llvm/test/CodeGen/X86/atomic-cmp-fold.ll | 46 ++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
index c3c1af48422b77..e2ce8d5f45922d 100644
--- a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
+++ b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
@@ -368,3 +368,49 @@ 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: movl %edi, %eax
+; X64-NEXT: cmpl (%rsi), %edi
+; X64-NEXT: adcl $0, %eax
+; X64-NEXT: retq
+;
+; X86-LABEL: add_ugt:
+; 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 %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 (%rsi), %edi
+; X64-NEXT: sbbl $0, %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: cmpl (%ecx), %eax
+; 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
+}
>From 2b00e430d03bd0cae11b0657b43fe2c69fbda385 Mon Sep 17 00:00:00 2001
From: HelloWorldU <asd001liu at gmail.com>
Date: Sat, 3 Oct 2026 18:47:07 +0800
Subject: [PATCH 2/2] [X86] Fix reversed condition in ADC/SBB fold of COND_A
compares
#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
---
llvm/lib/Target/X86/X86ISelLowering.cpp | 10 ----------
llvm/test/CodeGen/X86/atomic-cmp-fold.ll | 21 ++++++++++++++-------
2 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 7817c86155e5b0..971186abd73390 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 e2ce8d5f45922d..2cd181673517a7 100644
--- a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
+++ b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
@@ -374,17 +374,20 @@ define i1 @cmp32_volatile(ptr %p) {
define i32 @add_ugt(i32 %a, ptr %p) {
; X64-LABEL: add_ugt:
; X64: # %bb.0:
-; X64-NEXT: movl %edi, %eax
+; X64-NEXT: xorl %eax, %eax
; X64-NEXT: cmpl (%rsi), %edi
-; X64-NEXT: adcl $0, %eax
+; 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), %eax
-; X86-NEXT: cmpl (%ecx), %eax
-; X86-NEXT: adcl $0, %eax
+; 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
@@ -397,16 +400,20 @@ 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: sbbl $0, %eax
+; 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: sbbl $0, %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
More information about the llvm-commits
mailing list