[PATCH] D154533: [DAG] Improve carry reconstruction in combineCarryDiamond.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3542
   // Verify that the carry/borrow in is plausibly a carry/borrow bit.
-  // TODO: make getAsCarry() aware of how partial carries are merged.
-  if (CarryIn.getOpcode() != ISD::ZERO_EXTEND)
----------------
It does not look like this TODO was addressed, unless I'm misunderstanding it.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3548
 
   // Verify that the carry/borrow in is plausibly a carry/borrow bit.
+  CarryIn = getAsCarry(TLI, CarryIn, true);
----------------
It is now an arbitrary boolean value, not necessarily a carry/borrow.
I don't know whether this is good or bad, although it looks like it was indended to match only carry/borrow-producing nodes.
If go that road I'd also consider replacing this call with more general bit tracking (ComputeNumSignBits et al.).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154533