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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2842-2847
+  if (Carry0.getOperand(0) != Carry1.getValue(0)) {
+    if (Carry1.getOperand(0) != Carry0.getValue(0))
+      return SDValue();
+    // Canonicalize the carry in as Carry0 and the addition as Carry1.
+    std::swap(Carry0, Carry1);
+  }
----------------
I find this confusing.
Which one of Carry0 and Carry1 is the first one?
Can this be simplified to something like
```
// ???
if (Carry1.getOperand(0) != Carry0.getValue(0))
  std::swap(Carry0, Carry1);
if (Carry1.getOperand(0) != Carry0.getValue(0))
  return SDValue();
```
This would be more idiomatic..


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2852-2854
+  if (Carry0.getOperand(1).getOperand(0).getValueType() != MVT::i1)
+    return SDValue();
+
----------------
Do we care what `Carry0.getOperand(1)` is, what produces it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70079