[PATCH] D32925: [DAGCombine] (add/uaddo X, Carry) -> (addcarry X, 0, Carry)

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1985
+         (V.getOpcode() == ISD::AND && isOneConstant(V.getOperand(1))))
+      V = V.getOperand(0);
+
----------------
spatel wrote:
> deadalnix wrote:
> > RKSimon wrote:
> > > As was discussed on D32916, the carry is a boolean that will respect TargetLowering::BooleanContent so we can't just accept any extension - it has to match the bool type.
> > This just follow the chain to see if there is a carry generating ops at the end. This isn't generating anything, so I'm not sure what the problem is.
> In this case, we're taking a boolean carry value and using it as an operand for a new ADDCARRY node that has the same boolean content requirement, so I think this part is ok. 
> 
> Given that it's not clear from reading the code, please add a comment to explain the logic.
> 
> cc'ing @efriedma in case he sees a hole in that reasoning.
> 
> 
"X + (sext Carry)" clearly produces a different result from "X + (zext carry)"; I can't see how it makes sense to ignore the difference.


https://reviews.llvm.org/D32925