[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 a comment.

A few minors for you to look at



================
Comment at: include/llvm/CodeGen/ISDOpcodes.h:235
+    /// if the target supports it, as the carry is a regular value rather than
+    /// a glue, which allows further optimisation.
+    ADDCARRY, SUBCARRY,
----------------
Please detail the results as well (similar to ADDE/SUBE).


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8256
 
   // (trunc adde(X, Y, Carry)) -> (adde trunc(X), trunc(Y), Carry)
   // When the adde's carry is not used.
----------------
Update comment to describe addcarry too.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:517
+  SDValue Ops[3] = { N->getOperand(0), N->getOperand(1) };
+  auto C = N->getNumOperands();
+  assert(C <= 3 && "Too many operands");
----------------
This will be clearer with unsigned instead of auto


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1281
+  SDValue RHS = N->getOperand(1);
+  auto Carry = N->getOperand(2);
+  SDLoc DL(N);
----------------
Mixed use of SDValue and auto.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1768
+                                   ISD::ADDCARRY : ISD::SUBCARRY,
+                                 TLI.getTypeToExpandTo(*DAG.getContext(), NVT));
+  if (hasOPCARRY) {
----------------
clang-format


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1926
+                                 TLI.getTypeToExpandTo(*DAG.getContext(),
+                                                       LHS.getValueType()));
+
----------------
clang-format


https://reviews.llvm.org/D29872