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

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix added inline comments.


================
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);
----------------
barannikov88 wrote:
> 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.).
> 
In this case, we replace a complex web of instructions by just one add. Reconstructing a carry is worth it. The test cases that were changed bellow are good exemple of that, the carry is passed as an argument to the functions, so it is not possible to know if these are "real" carries or not, and it is nevertheless worth doing the transform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154533