[llvm] fb9c9eb - [X86][GISel] Fix carry-in for selectUAddSub. (#199261)
via llvm-commits
llvm-commits at lists.llvm.org
Sun May 24 09:22:56 PDT 2026
Author: Justin Lebar
Date: 2026-05-24T09:22:52-07:00
New Revision: fb9c9eb9d2d97cff5f19fbb0f9a7828afb4c4271
URL: https://github.com/llvm/llvm-project/commit/fb9c9eb9d2d97cff5f19fbb0f9a7828afb4c4271
DIFF: https://github.com/llvm/llvm-project/commit/fb9c9eb9d2d97cff5f19fbb0f9a7828afb4c4271.diff
LOG: [X86][GISel] Fix carry-in for selectUAddSub. (#199261)
When G_UADDE/G_USUBE was chained off a previous G_UADDE/G_UADDO/
G_USUBE/G_USUBO, selectUAddSub re-materialized EFLAGS.CF from the
previous SETB byte using CMP r, 1. That computes (r - 1) and sets
CF iff r < 1 unsigned, i.e. CF = (r == 0) -- the inverse of the
desired carry. The following ADC/SBB then consumed the wrong CF and
produced an off-by-one upper word; e.g. `add i128 0xFF..FF, 1` under
-global-isel returned hi=0 lo=0 instead of hi=1 lo=0.
Emit NEG r instead: NEG sets CF iff its operand is non-zero, matching
the SETB byte. NEG is a two-address (tied) instruction, so emit it
into a fresh virtual register rather than redefining the carry-in
vreg.
C reproducer (compile on x86_64-linux-gnu and run):
```
// clang -O2 -fglobal-isel repro.c -o repro && ./repro
#include <stdio.h>
__attribute__((noinline))
__int128 add128(__int128 a, __int128 b) { return a + b; }
int main(void) {
__int128 a = (__int128)0xFFFFFFFFFFFFFFFFULL;
__int128 r = add128(a, 1);
unsigned long long lo = r, hi = r >> 64;
printf("hi=0x%llx lo=0x%llx\n", hi, lo);
return (hi == 1 && lo == 0) ? 0 : 1;
}
```
Without -fglobal-isel: prints "hi=0x1 lo=0x0", exits 0.
With -fglobal-isel: prints "hi=0x0 lo=0x0", exits 1.
The buggy code-gen emits the inverted-carry sequence (clang trunk,
pre-fix):
```
add128:
movq %rdx, %rax
addq %rdi, %rax
setb %dl
cmpb $1, %dl # CF = (dl == 0), inverse of intended
adcq %rsi, %rcx
movq %rcx, %rdx
retq
```
This bug was found by a large run of Opus 4.7 looking for bugs in
LLVM.
Added:
Modified:
llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
llvm/test/CodeGen/X86/GlobalISel/add-scalar.ll
llvm/test/CodeGen/X86/GlobalISel/select-add-x32.mir
llvm/test/CodeGen/X86/GlobalISel/sub-scalar.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
index a77049fba133e..3c94c33cdaaa6 100644
--- a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
+++ b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
@@ -1272,15 +1272,16 @@ bool X86InstructionSelector::selectUAddSub(MachineInstr &I,
Def->getOpcode() == TargetOpcode::G_UADDO ||
Def->getOpcode() == TargetOpcode::G_USUBE ||
Def->getOpcode() == TargetOpcode::G_USUBO) {
- // carry set by prev ADD/SUB.
-
- BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(X86::CMP8ri))
- .addReg(CarryInReg)
- .addImm(1);
-
+ // The carry-in is a SETB byte (0 or 1) from a chained add/sub.
+ // Materialize EFLAGS.CF from that byte for the following ADC/SBB
+ // by emitting NEG, which sets CF iff its operand is non-zero.
if (!RBI.constrainGenericRegister(CarryInReg, *CarryRC, MRI))
return false;
+ Register NegDef = MRI.createVirtualRegister(CarryRC);
+ BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(X86::NEG8r), NegDef)
+ .addReg(CarryInReg);
+
Opcode = IsSub ? OpSBB : OpADC;
} else if (auto val = getIConstantVRegVal(CarryInReg, MRI)) {
// carry is constant, support only 0.
diff --git a/llvm/test/CodeGen/X86/GlobalISel/add-scalar.ll b/llvm/test/CodeGen/X86/GlobalISel/add-scalar.ll
index 7cdfd519f04dc..3a114222c0795 100644
--- a/llvm/test/CodeGen/X86/GlobalISel/add-scalar.ll
+++ b/llvm/test/CodeGen/X86/GlobalISel/add-scalar.ll
@@ -8,7 +8,7 @@ define i128 @test_add_i128(i128 %arg1, i128 %arg2) nounwind {
; X64-NEXT: movq %rdx, %rax
; X64-NEXT: addq %rdi, %rax
; X64-NEXT: setb %dl
-; X64-NEXT: cmpb $1, %dl
+; X64-NEXT: negb %dl
; X64-NEXT: adcq %rsi, %rcx
; X64-NEXT: movq %rcx, %rdx
; X64-NEXT: retq
@@ -25,13 +25,13 @@ define i128 @test_add_i128(i128 %arg1, i128 %arg2) nounwind {
; X86-NEXT: movl {{[0-9]+}}(%esp), %edi
; X86-NEXT: addl {{[0-9]+}}(%esp), %ecx
; X86-NEXT: setb %bl
-; X86-NEXT: cmpb $1, %bl
+; X86-NEXT: negb %bl
; X86-NEXT: adcl {{[0-9]+}}(%esp), %edx
; X86-NEXT: setb %bl
-; X86-NEXT: cmpb $1, %bl
+; X86-NEXT: negb %bl
; X86-NEXT: adcl {{[0-9]+}}(%esp), %esi
; X86-NEXT: setb %bl
-; X86-NEXT: cmpb $1, %bl
+; X86-NEXT: negb %bl
; X86-NEXT: adcl {{[0-9]+}}(%esp), %edi
; X86-NEXT: movl %ecx, (%eax)
; X86-NEXT: movl %edx, 4(%eax)
@@ -57,7 +57,7 @@ define i64 @test_add_i64(i64 %arg1, i64 %arg2) {
; X86-NEXT: movl {{[0-9]+}}(%esp), %edx
; X86-NEXT: addl {{[0-9]+}}(%esp), %eax
; X86-NEXT: setb %cl
-; X86-NEXT: cmpb $1, %cl
+; X86-NEXT: negb %cl
; X86-NEXT: adcl {{[0-9]+}}(%esp), %edx
; X86-NEXT: retl
%ret = add i64 %arg1, %arg2
diff --git a/llvm/test/CodeGen/X86/GlobalISel/select-add-x32.mir b/llvm/test/CodeGen/X86/GlobalISel/select-add-x32.mir
index b85180fb467eb..c2a93fab79d0a 100644
--- a/llvm/test/CodeGen/X86/GlobalISel/select-add-x32.mir
+++ b/llvm/test/CodeGen/X86/GlobalISel/select-add-x32.mir
@@ -32,7 +32,7 @@ body: |
; X32-NEXT: [[DEF3:%[0-9]+]]:gr32 = IMPLICIT_DEF
; X32-NEXT: [[ADD32rr:%[0-9]+]]:gr32 = ADD32rr [[DEF]], [[DEF2]], implicit-def $eflags
; X32-NEXT: [[SETCCr:%[0-9]+]]:gr8 = SETCCr 2, implicit $eflags
- ; X32-NEXT: CMP8ri [[SETCCr]], 1, implicit-def $eflags
+ ; X32-NEXT: [[NEG8r:%[0-9]+]]:gr8 = NEG8r [[SETCCr]], implicit-def $eflags
; X32-NEXT: [[ADC32rr:%[0-9]+]]:gr32 = ADC32rr [[DEF1]], [[DEF3]], implicit-def $eflags, implicit $eflags
; X32-NEXT: [[SETCCr1:%[0-9]+]]:gr8 = SETCCr 2, implicit $eflags
; X32-NEXT: $eax = COPY [[ADD32rr]]
diff --git a/llvm/test/CodeGen/X86/GlobalISel/sub-scalar.ll b/llvm/test/CodeGen/X86/GlobalISel/sub-scalar.ll
index 825e7bb064519..a791b53c2a76c 100644
--- a/llvm/test/CodeGen/X86/GlobalISel/sub-scalar.ll
+++ b/llvm/test/CodeGen/X86/GlobalISel/sub-scalar.ll
@@ -8,7 +8,7 @@ define i128 @test_sub_i128(i128 %arg1, i128 %arg2) nounwind {
; X64-NEXT: movq %rdi, %rax
; X64-NEXT: subq %rdx, %rax
; X64-NEXT: setb %dl
-; X64-NEXT: cmpb $1, %dl
+; X64-NEXT: negb %dl
; X64-NEXT: sbbq %rcx, %rsi
; X64-NEXT: movq %rsi, %rdx
; X64-NEXT: retq
@@ -25,13 +25,13 @@ define i128 @test_sub_i128(i128 %arg1, i128 %arg2) nounwind {
; X86-NEXT: movl {{[0-9]+}}(%esp), %edi
; X86-NEXT: subl {{[0-9]+}}(%esp), %ecx
; X86-NEXT: setb %bl
-; X86-NEXT: cmpb $1, %bl
+; X86-NEXT: negb %bl
; X86-NEXT: sbbl {{[0-9]+}}(%esp), %edx
; X86-NEXT: setb %bl
-; X86-NEXT: cmpb $1, %bl
+; X86-NEXT: negb %bl
; X86-NEXT: sbbl {{[0-9]+}}(%esp), %esi
; X86-NEXT: setb %bl
-; X86-NEXT: cmpb $1, %bl
+; X86-NEXT: negb %bl
; X86-NEXT: sbbl {{[0-9]+}}(%esp), %edi
; X86-NEXT: movl %ecx, (%eax)
; X86-NEXT: movl %edx, 4(%eax)
@@ -58,7 +58,7 @@ define i64 @test_sub_i64(i64 %arg1, i64 %arg2) {
; X86-NEXT: movl {{[0-9]+}}(%esp), %edx
; X86-NEXT: subl {{[0-9]+}}(%esp), %eax
; X86-NEXT: setb %cl
-; X86-NEXT: cmpb $1, %cl
+; X86-NEXT: negb %cl
; X86-NEXT: sbbl {{[0-9]+}}(%esp), %edx
; X86-NEXT: retl
%ret = sub i64 %arg1, %arg2
More information about the llvm-commits
mailing list