[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: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:516
   EVT ValueVTs[] = { N->getValueType(0), NVT };
-  SDValue Ops[] = { N->getOperand(0), N->getOperand(1) };
+  SDValue Ops[3] = { N->getOperand(0), N->getOperand(1) };
+  unsigned C = N->getNumOperands();
----------------
Add explicit SDValue() to initialize Ops[2] ?


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1832
+    EVT OvfVT = getSetCCResultType(NVT);
+    SDVTList VTList = DAG.getVTList(NVT, OvfVT);
     TargetLoweringBase::BooleanContent BoolType = TLI.getBooleanContents(NVT);
----------------
Should this be a separate fix?


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1934
+void DAGTypeLegalizer::ExpandIntRes_UADDSUBO(SDNode *N,
+                                             SDValue &Lo, SDValue &Hi) {
+  SDValue LHS = N->getOperand(0);
----------------
Should this be added separately? At very least DAGTypeLegalizer::ExpandIntRes_UADDSUBO should be moved as an NFC pre-commit so that diff is more obvious.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:34407
+// Optimize RES, EFLAGS = X86ISD::ADD LHS, RHS
+static SDValue combineADD(SDNode *N, SelectionDAG &DAG,
+                          X86TargetLowering::DAGCombinerInfo &DCI) {
----------------
We already have a combineAdd function - this should be named something like combineX86Add


https://reviews.llvm.org/D29872