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

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1902
+  return SDValue();
+}
+
----------------
RKSimon wrote:
> Split this into a separate small patch + tests?
No you get a bunch of regression without these. This is doing the same transforms as for ADDC, so this isn't actually new.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2517
+    }
+    LLVM_FALLTHROUGH;
   case ISD::ADD:
----------------
RKSimon wrote:
> Could the ISD::UADDO case be split into a separate small patch + tests?
Same thing, bunch of regression without this.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33951
+  return SDValue();
+}
+
----------------
RKSimon wrote:
> Again, can this be extracted into its own patch + tests?
Without this, you get an add with a carry, followed by a setb or an sbb instruction to materialise the carry, followed by an add Carry, -1 to regenerate the carry. It's pretty bad.


https://reviews.llvm.org/D29872