[X86] Merge ISD::ADD/SUB nodes into X86ISD::ADD/SUB equivalents (PR40483)

[RKSimon]

Summary

Avoid ADD/SUB instruction duplication by reusing the X86ISD::ADD/SUB results.

Includes ADD commutation - I tried to include NEG+SUB SUB commutation as well but this causes regressions as we don't have good combine coverage to simplify X86ISD::SUB.

[craig.topper]

LGTM with one minor

lib/Target/X86/X86ISelLowering.cpp

41001 ↗

(On Diff #188091)

Why not just pass SDValue(N, 0) in the CombineTo call?

[RKSimon]

Cheers Craig

lib/Target/X86/X86ISelLowering.cpp

41001 ↗

(On Diff #188091)

Legacy from when I was trying to get the negation to work - I'll move it back.

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

static SDValue combineCMP(SDNode *N, SelectionDAG &DAG) {
   if (NewOpc == X86ISD::AND)
     return DAG.getNode(X86ISD::CMP, dl, MVT::i32, Op,
                        DAG.getConstant(0, dl, VT));
 
   // Return the flags.
   return Op.getValue(1);
 }
 
-static SDValue combineX86AddSub(SDNode *N, SelectionDAG &DAG) {
+static SDValue combineX86AddSub(SDNode *N, SelectionDAG &DAG,
+                                TargetLowering::DAGCombinerInfo &DCI) {
   assert((X86ISD::ADD == N->getOpcode() || X86ISD::SUB == N->getOpcode()) &&
          "Expected X86ISD::ADD or X86ISD::SUB");
 
-  // If we don't use the flag result, simplify back to a simple ADD/SUB.
-  if (N->hasAnyUseOfValue(1))
-    return SDValue();
-
-  SDLoc DL(N);
   SDValue LHS = N->getOperand(0);
   SDValue RHS = N->getOperand(1);
-  SDValue Res = DAG.getNode(X86ISD::ADD == N->getOpcode() ? ISD::ADD : ISD::SUB,
-                            DL, LHS.getSimpleValueType(), LHS, RHS);
+  MVT VT = LHS.getSimpleValueType();
+  unsigned GenericOpc = X86ISD::ADD == N->getOpcode() ? ISD::ADD : ISD::SUB;
+
+  // If we don't use the flag result, simplify back to a generic ADD/SUB.
+  if (!N->hasAnyUseOfValue(1)) {
+    SDLoc DL(N);
+    SDValue Res = DAG.getNode(GenericOpc, DL, VT, LHS, RHS);
     return DAG.getMergeValues({Res, DAG.getConstant(0, DL, MVT::i32)}, DL);
   }
 
+  // Fold any similar generic ADD/SUB opcodes to reuse this node.
+  auto MatchGeneric = [&](SDValue N0, SDValue N1, bool Negate) {
+    // TODO: Add SUB(RHS, LHS) -> SUB(0, SUB(LHS, RHS)) negation support, this
+    // currently causes regressions as we don't have broad x86sub combines.
+    if (Negate)
+      return;
+    SDValue Ops[] = {N0, N1};
+    SDVTList VTs = DAG.getVTList(N->getValueType(0));
+    if (SDNode *GenericAddSub = DAG.getNodeIfExists(GenericOpc, VTs, Ops))
+      DCI.CombineTo(GenericAddSub, SDValue(N, 0));
+  };
+  MatchGeneric(LHS, RHS, false);
+  MatchGeneric(RHS, LHS, X86ISD::SUB == N->getOpcode());
+
+  return SDValue();
+}
+
 static SDValue combineSBB(SDNode *N, SelectionDAG &DAG) {
   if (SDValue Flags = combineCarryThroughADD(N->getOperand(2), DAG)) {
     MVT VT = N->getSimpleValueType(0);
     SDVTList VTs = DAG.getVTList(VT, MVT::i32);
     return DAG.getNode(X86ISD::SBB, SDLoc(N), VTs,
                        N->getOperand(0), N->getOperand(1),
                        Flags);
   }
SDValue X86TargetLowering::PerformDAGCombine(SDNode *N,
   case ISD::SELECT:
   case X86ISD::BLENDV:      return combineSelect(N, DAG, DCI, Subtarget);
   case ISD::BITCAST:        return combineBitcast(N, DAG, DCI, Subtarget);
   case X86ISD::CMOV:        return combineCMov(N, DAG, DCI, Subtarget);
   case X86ISD::CMP:         return combineCMP(N, DAG);
   case ISD::ADD:            return combineAdd(N, DAG, Subtarget);
   case ISD::SUB:            return combineSub(N, DAG, Subtarget);
   case X86ISD::ADD:
-  case X86ISD::SUB:         return combineX86AddSub(N, DAG);
+  case X86ISD::SUB:         return combineX86AddSub(N, DAG, DCI);
   case X86ISD::SBB:         return combineSBB(N, DAG);
   case X86ISD::ADC:         return combineADC(N, DAG, DCI);
   case ISD::MUL:            return combineMul(N, DAG, DCI, Subtarget);
   case ISD::SHL:
   case ISD::SRA:
   case ISD::SRL:            return combineShift(N, DAG, DCI, Subtarget);
   case ISD::AND:            return combineAnd(N, DAG, DCI, Subtarget);
   case ISD::OR:             return combineOr(N, DAG, DCI, Subtarget);

llvm/trunk/test/CodeGen/X86/combine-adc.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-unknown-unknown | FileCheck %s --check-prefixes=X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s --check-prefixes=X64
 
 define i32 @PR40483_add1(i32*, i32) nounwind {
 ; X86-LABEL: PR40483_add1:
 ; X86:       # %bb.0:
-; X86-NEXT:    pushl %esi
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    movl (%edx), %esi
-; X86-NEXT:    leal (%esi,%ecx), %eax
-; X86-NEXT:    addl %ecx, %esi
-; X86-NEXT:    movl %esi, (%edx)
-; X86-NEXT:    jae .LBB0_1
-; X86-NEXT:  # %bb.2:
+; X86-NEXT:    movl (%ecx), %eax
+; X86-NEXT:    addl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl %eax, (%ecx)
+; X86-NEXT:    jae .LBB0_2
+; X86-NEXT:  # %bb.1:
 ; X86-NEXT:    xorl %eax, %eax
-; X86-NEXT:    popl %esi
-; X86-NEXT:    retl
-; X86-NEXT:  .LBB0_1:
-; X86-NEXT:    orl %eax, %eax
-; X86-NEXT:    popl %esi
+; X86-NEXT:  .LBB0_2:
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: PR40483_add1:
 ; X64:       # %bb.0:
-; X64-NEXT:    # kill: def $esi killed $esi def $rsi
-; X64-NEXT:    movl (%rdi), %ecx
-; X64-NEXT:    leal (%rcx,%rsi), %edx
-; X64-NEXT:    orl %edx, %edx
 ; X64-NEXT:    xorl %eax, %eax
-; X64-NEXT:    addl %esi, %ecx
-; X64-NEXT:    movl %ecx, (%rdi)
-; X64-NEXT:    cmovael %edx, %eax
+; X64-NEXT:    addl (%rdi), %esi
+; X64-NEXT:    movl %esi, (%rdi)
+; X64-NEXT:    cmovael %esi, %eax
 ; X64-NEXT:    retq
   %3 = load i32, i32* %0, align 8
   %4 = tail call { i8, i32 } @llvm.x86.addcarry.32(i8 0, i32 %3, i32 %1)
   %5 = extractvalue { i8, i32 } %4, 1
   store i32 %5, i32* %0, align 8
   %6 = extractvalue { i8, i32 } %4, 0
   %7 = icmp eq i8 %6, 0
   %8 = add i32 %1, %3
   %9 = or i32 %5, %8
   %10 = select i1 %7, i32 %9, i32 0
   ret i32 %10
 }
 
 define i32 @PR40483_add2(i32*, i32) nounwind {
 ; X86-LABEL: PR40483_add2:
 ; X86:       # %bb.0:
-; X86-NEXT:    pushl %edi
-; X86-NEXT:    pushl %esi
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %esi
-; X86-NEXT:    movl (%esi), %edi
-; X86-NEXT:    leal (%edi,%edx), %ecx
+; X86-NEXT:    movl (%edx), %ecx
 ; X86-NEXT:    xorl %eax, %eax
-; X86-NEXT:    addl %edx, %edi
-; X86-NEXT:    movl %edi, (%esi)
+; X86-NEXT:    addl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl %ecx, (%edx)
 ; X86-NEXT:    jae .LBB1_2
 ; X86-NEXT:  # %bb.1:
-; X86-NEXT:    orl %ecx, %ecx
 ; X86-NEXT:    movl %ecx, %eax
 ; X86-NEXT:  .LBB1_2:
-; X86-NEXT:    popl %esi
-; X86-NEXT:    popl %edi
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: PR40483_add2:
 ; X64:       # %bb.0:
-; X64-NEXT:    # kill: def $esi killed $esi def $rsi
-; X64-NEXT:    movl (%rdi), %ecx
-; X64-NEXT:    leal (%rcx,%rsi), %edx
-; X64-NEXT:    orl %edx, %edx
 ; X64-NEXT:    xorl %eax, %eax
-; X64-NEXT:    addl %esi, %ecx
-; X64-NEXT:    movl %ecx, (%rdi)
-; X64-NEXT:    cmovbl %edx, %eax
+; X64-NEXT:    addl (%rdi), %esi
+; X64-NEXT:    movl %esi, (%rdi)
+; X64-NEXT:    cmovbl %esi, %eax
 ; X64-NEXT:    retq
   %3 = load i32, i32* %0, align 8
   %4 = tail call { i8, i32 } @llvm.x86.addcarry.32(i8 0, i32 %3, i32 %1)
   %5 = extractvalue { i8, i32 } %4, 1
   store i32 %5, i32* %0, align 8
   %6 = extractvalue { i8, i32 } %4, 0
   %7 = icmp eq i8 %6, 0
   %8 = add i32 %3, %1
   %9 = or i32 %5, %8
   %10 = select i1 %7, i32 0, i32 %9
   ret i32 %10
 }
 
 declare { i8, i32 } @llvm.x86.addcarry.32(i8, i32, i32)

llvm/trunk/test/CodeGen/X86/combine-sbb.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-unknown-unknown | FileCheck %s --check-prefixes=X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s --check-prefixes=X64
 
 %WideUInt32 = type { i32, i32 }
 
 define void @PR25858_i32(%WideUInt32* sret, %WideUInt32*, %WideUInt32*) nounwind {
 ; X86-LABEL: PR25858_i32:
 ; X86:       # %bb.0: # %top
 ; X86-NEXT:    pushl %esi
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    movl (%ecx), %esi
-; X86-NEXT:    movl 4(%ecx), %ecx
-; X86-NEXT:    subl (%edx), %esi
-; X86-NEXT:    sbbl 4(%edx), %ecx
-; X86-NEXT:    movl %ecx, 4(%eax)
+; X86-NEXT:    movl (%edx), %esi
+; X86-NEXT:    movl 4(%edx), %edx
+; X86-NEXT:    subl (%ecx), %esi
+; X86-NEXT:    sbbl 4(%ecx), %edx
+; X86-NEXT:    movl %edx, 4(%eax)
 ; X86-NEXT:    movl %esi, (%eax)
 ; X86-NEXT:    popl %esi
 ; X86-NEXT:    retl $4
 ;
 ; X64-LABEL: PR25858_i32:
 ; X64:       # %bb.0: # %top
 ; X64-NEXT:    movq %rdi, %rax
 ; X64-NEXT:    movl (%rsi), %ecx
; X64-NEXT:    retq
   %9 = or i32 %5, %8
   %10 = select i1 %7, i32 %9, i32 0
   ret i32 %10
 }
 
 define i32 @PR40483_sub4(i32*, i32) nounwind {
 ; X86-LABEL: PR40483_sub4:
 ; X86:       # %bb.0:
-; X86-NEXT:    pushl %edi
-; X86-NEXT:    pushl %esi
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %esi
-; X86-NEXT:    movl (%esi), %edi
-; X86-NEXT:    movl %edi, %ecx
-; X86-NEXT:    subl %edx, %ecx
+; X86-NEXT:    movl (%edx), %ecx
 ; X86-NEXT:    xorl %eax, %eax
-; X86-NEXT:    subl %edx, %edi
-; X86-NEXT:    movl %edi, (%esi)
+; X86-NEXT:    subl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    movl %ecx, (%edx)
 ; X86-NEXT:    jae .LBB6_2
 ; X86-NEXT:  # %bb.1:
-; X86-NEXT:    orl %ecx, %ecx
 ; X86-NEXT:    movl %ecx, %eax
 ; X86-NEXT:  .LBB6_2:
-; X86-NEXT:    popl %esi
-; X86-NEXT:    popl %edi
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: PR40483_sub4:
 ; X64:       # %bb.0:
-; X64-NEXT:    movl (%rdi), %ecx
-; X64-NEXT:    movl %ecx, %edx
-; X64-NEXT:    subl %esi, %edx
-; X64-NEXT:    orl %edx, %edx
-; X64-NEXT:    xorl %eax, %eax
-; X64-NEXT:    subl %esi, %ecx
-; X64-NEXT:    movl %ecx, (%rdi)
-; X64-NEXT:    cmovbl %edx, %eax
+; X64-NEXT:    movl (%rdi), %eax
+; X64-NEXT:    xorl %ecx, %ecx
+; X64-NEXT:    subl %esi, %eax
+; X64-NEXT:    movl %eax, (%rdi)
+; X64-NEXT:    cmovael %ecx, %eax
 ; X64-NEXT:    retq
   %3 = load i32, i32* %0, align 8
   %4 = tail call { i8, i32 } @llvm.x86.subborrow.32(i8 0, i32 %3, i32 %1)
   %5 = extractvalue { i8, i32 } %4, 1
   store i32 %5, i32* %0, align 8
   %6 = extractvalue { i8, i32 } %4, 0
   %7 = icmp eq i8 %6, 0
   %8 = sub i32 %3, %1
   %9 = or i32 %5, %8
   %10 = select i1 %7, i32 0, i32 %9
   ret i32 %10
 }
 
 declare { i8, i32 } @llvm.x86.subborrow.32(i8, i32, i32)