[PATCH] D29872: Do not legalize large add with addc/adde, introduce addcarry and do it with uaddo/addcarry

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: include/llvm/CodeGen/ISDOpcodes.h:215-218
     /// Carry-setting nodes for multiple precision addition and subtraction.
     /// These nodes take two operands of the same value type, and produce two
     /// results.  The first result is the normal add or sub result, the second
     /// result is the carry flag result.
----------------
spatel wrote:
> If I'd just stumbled into this code, I'd be confused why we have ADDC and ADDE and ADDCARRY. If the intent is to remove these, please add a 'FIXME' comment here that says we should get rid of these.
I agree with Sanjay - you need to document the plan for ADDC/SUBC here


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1941
+
+  bool hasOPCARRY = TLI.isOperationLegalOrCustom(
+      N->getOpcode() == ISD::ADD ? ISD::ADDCARRY : ISD::SUBCARRY,
----------------
mkazantsev wrote:
> hasOpCarry?
HasOpCarry (style)


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1954
+
+    auto Opc = N->getOpcode() == ISD::UADDO ? ISD::ADDCARRY : ISD::SUBCARRY;
+    Lo = DAG.getNode(N->getOpcode(), dl, VTList, LoOps);
----------------
Please unsigned instead of auto for the simple types


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1987
+  SDValue LoOps[3] = { LHSL, RHSL, N->getOperand(2) };
+  SDValue HiOps[3] = { LHSH, RHSH };
+
----------------
SDValue HiOps[3] = { LHSH, RHSH, SDValue() };


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:23359
+static SDValue LowerADDSUBCARRY(SDValue Op, SelectionDAG &DAG) {
+  auto N = Op.getNode();
+  MVT VT = N->getSimpleValueType(0);
----------------
unsigned


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:23377
+  unsigned Opc = Op.getOpcode() == ISD::ADDCARRY ? X86ISD::ADC : X86ISD::SBB;
+  auto Sum = DAG.getNode(Opc, DL, VTs, Op.getOperand(0),
+                         Op.getOperand(1), Carry.getValue(1));
----------------
SDValue


https://reviews.llvm.org/D29872