[X86] Optimized ADD + ADC to ADC

[JaydeepChauhan14]

This patch folds an adc followed by an add into a single adc instruction when adding constants.

Fixes #173408

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: None (JaydeepChauhan14)

Changes

• This PR fixing the [X86] Missed combining of adc + add with constant arguments #173408 issue.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+7)
• (modified) llvm/test/CodeGen/X86/combine-adc.ll (+30)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 811ffb090d751..fd6e7da196d2f 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -58164,6 +58164,13 @@ static SDValue combineX86AddSub(SDNode *N, SelectionDAG &DAG,
     }
   }
 
+  // Fold ADD(ADC(Y,0,CF), C) -> ADC(Y, C, CF)
+  if (!IsSub && LHS.getOpcode() == X86ISD::ADC &&
+      X86::isZeroNode(LHS.getOperand(1)) && isa<ConstantSDNode>(RHS)) {
+    return DAG.getNode(X86ISD::ADC, DL, N->getVTList(), LHS.getOperand(0), RHS,
+                       LHS.getOperand(2));
+  }
+
   // TODO: Can we drop the ZeroSecondOpOnly limit? This is to guarantee that the
   // EFLAGS result doesn't change.
   return combineAddOrSubToADCOrSBB(IsSub, DL, VT, LHS, RHS, DAG,
diff --git a/llvm/test/CodeGen/X86/combine-adc.ll b/llvm/test/CodeGen/X86/combine-adc.ll
index a2aaea31aa6ff..03e60c10cec4b 100644
--- a/llvm/test/CodeGen/X86/combine-adc.ll
+++ b/llvm/test/CodeGen/X86/combine-adc.ll
@@ -136,5 +136,35 @@ define i32 @adc_merge_sub(i32 %a0) nounwind {
   ret i32 %result
 }
 
+define i32 @optimize_adc(i32 %0, i32 %1, i32 %2) {
+; X86-LABEL: optimize_adc:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    cmpl %eax, %ecx
+; X86-NEXT:    adcl $42, %edx
+; X86-NEXT:    js .LBB4_2
+; X86-NEXT:  # %bb.1:
+; X86-NEXT:    movl %ecx, %eax
+; X86-NEXT:  .LBB4_2:
+; X86-NEXT:    retl
+;
+; X64-LABEL: optimize_adc:
+; X64:       # %bb.0:
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    cmpl %esi, %edi
+; X64-NEXT:    adcl $42, %edx
+; X64-NEXT:    cmovsl %esi, %eax
+; X64-NEXT:    retq
+  %4 = icmp ult i32 %0, %1
+  %5 = zext i1 %4 to i32
+  %6 = add i32 %2, 42
+  %7 = add i32 %6, %5
+  %8 = icmp slt i32 %7, 0
+  %9 = select i1 %8, i32 %1, i32 %0
+  ret i32 %9
+}
+
 declare { i8, i32 } @llvm.x86.addcarry.32(i8, i32, i32)
 declare void @use(i8)

[zwuis]

• This PR fixing the [X86] Missed combining of adc + add with constant arguments #173408 issue.

You could use syntax like fixes #173408 so that the issue is automatically closed when the PR is merged.

[JaydeepChauhan14]

For transformation ADD( ADC(x, c1, CF), c2 ) == ADC(x, c1+c2, CF),
Tried test/values with Brute force state and equivalence validation as given some below values. eflags values are not same after combine for many cases/values.
Then it is safe to implement this transformation...? Please share your view for the same.

Mismatch i=2 cf=1 x=7fffffff c1=00000000 c2=7fffffff
Before Combine: out=ffffffff eflags=00000084
After Combine: out=ffffffff eflags=00000894 (k=7fffffff)
Mismatch i=3 cf=0 x=80000000 c1=80000000 c2=00000000
Before Combine: out=00000000 eflags=00000044
After Combine: out=00000000 eflags=00000845 (k=80000000)
Mismatch i=3 cf=1 x=80000000 c1=80000000 c2=00000000
Before Combine: out=00000001 eflags=00000000
After Combine: out=00000001 eflags=00000801 (k=80000000)

[UniQP]

For transformation ADD( ADC(x, c1, CF), c2 ) == ADC(x, c1+c2, CF), Tried test/values with Brute force state and equivalence validation as given some below values. eflags values are not same after combine for many cases/values. Then it is safe to implement this transformation...? Please share your view for the same.

Mismatch i=2 cf=1 x=7fffffff c1=00000000 c2=7fffffff
Before Combine: out=ffffffff eflags=00000084
After Combine: out=ffffffff eflags=00000894 (k=7fffffff)
Mismatch i=3 cf=0 x=80000000 c1=80000000 c2=00000000
Before Combine: out=00000000 eflags=00000044
After Combine: out=00000000 eflags=00000845 (k=80000000)
Mismatch i=3 cf=1 x=80000000 c1=80000000 c2=00000000
Before Combine: out=00000001 eflags=00000000
After Combine: out=00000001 eflags=00000801 (k=80000000)

The transformation is safe if the carry and overflow flags are unused (which you already check via needCarryOrOverflowFlag). You need to add the same check to the other optimization though (see #173543 (comment)).

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[JaydeepChauhan14]

Ping @phoebewang, @topperc, @RKSimon

[UniQP]

#173543 (comment) is not addressed yet.

[JaydeepChauhan14]

Ping @topperc, @RKSimon, @phoebewang

[phoebewang]

LGTM.

[JaydeepChauhan14]

Ping @topperc, @RKSimon

[topperc]

I think the description can use more detail so you don't have to follow a link to understand the intent of the patch.

[JaydeepChauhan14]

I think the description can use more detail so you don't have to follow a link to understand the intent of the patch.

Done

[topperc]

LGTM

[github-actions]

🐧 Linux x64 Test Results

• 192008 tests passed
• 4912 tests skipped

✅ The build succeeded and all tests passed.

[JaydeepChauhan14]

Ping @RKSimon

[RKSimon]

LGTM