[PATCH] D70079: [SelectionDAG] Combine U{ADD,SUB}O diamonds into {ADD,SUB}CARRY

[David Zarzycki via Phabricator via llvm-commits]

davezarzycki marked 13 inline comments as done.
davezarzycki added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2846
+
+  // Canonicalize the carry in as Carry0 and the add/sub of A and B as Carry1.
+  // I.e. the middle and top nodes respectively in the above ASCII art.
----------------
lebedev.ri wrote:
> Ah, so the numbering is from bottom to top, i expected the other way around.
I've swapped the ordering to match your expectations.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2861-2865
+  // Verify that the carry/borrow in is plausibly a carry/borrow bit.
+  if (Carry0.getOperand(1).getOpcode() != ISD::ZERO_EXTEND)
+    return SDValue();
+  if (Carry0.getOperand(1).getOperand(0).getValueType() != MVT::i1)
+    return SDValue();
----------------
lebedev.ri wrote:
> There's `getAsCarry()` should it be used here, or are we okay with some other `i1`?
> (can we have some other i1 by now?)
That function is non-recursive, so it doesn't work. In other words, it doesn't know how to handle carry flags being merged. I can change that if people are okay with that. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70079/new/

https://reviews.llvm.org/D70079