[llvm] [X86] Swap CMP operands for COND_A in ADC/SBB fold (PR #228710)
Aochang Liu via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 3 06:47:20 PDT 2026
https://github.com/HelloWorldU created https://github.com/llvm/llvm-project/pull/228710
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
>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/3] [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/3] [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
>From 139ab5f6fec521a654b1bb85dd32434c6c2b0e77 Mon Sep 17 00:00:00 2001
From: HelloWorldU <asd001liu at gmail.com>
Date: Sat, 3 Oct 2026 21:43:01 +0800
Subject: [PATCH 3/3] [X86] Swap CMP operands for COND_A in ADC/SBB fold
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
---
llvm/lib/Target/X86/X86ISelLowering.cpp | 15 ++---
llvm/test/CodeGen/X86/atomic-cmp-fold.ll | 78 ++++++++++++++++++------
2 files changed, 66 insertions(+), 27 deletions(-)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 971186abd73390..a2923e134b4111 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -53931,17 +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.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 2cd181673517a7..74306755b0b114 100644
--- a/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
+++ b/llvm/test/CodeGen/X86/atomic-cmp-fold.ll
@@ -369,25 +369,22 @@ define i1 @cmp32_volatile(ptr %p) {
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).
+; 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: xorl %eax, %eax
-; X64-NEXT: cmpl (%rsi), %edi
-; X64-NEXT: seta %al
-; X64-NEXT: addl %edi, %eax
+; 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: 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: 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
@@ -400,20 +397,16 @@ 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: cmpl %edi, (%rsi)
+; 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: xorl %edx, %edx
-; X86-NEXT: cmpl (%ecx), %eax
-; X86-NEXT: seta %dl
-; X86-NEXT: subl %edx, %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
@@ -421,3 +414,48 @@ define i32 @sub_ugt(i32 %a, ptr %p) {
%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
+}
More information about the llvm-commits
mailing list