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

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1985
+         (V.getOpcode() == ISD::AND && isOneConstant(V.getOperand(1))))
+      V = V.getOperand(0);
+
----------------
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.


================
Comment at: test/CodeGen/X86/add-of-carry.ll:16
+; CHECK-NEXT:    addl %ecx, %edx
+; CHECK-NEXT:    adcl %ecx, %eax
 ; CHECK-NEXT:    retl
----------------
RKSimon wrote:
> This is annoying....
I have a patch making this better. However, it has to be noted that this code is very unrepresentative from code in the field, as it essentially does a + b + carry(a, b) . This is fine to test this as it exercise edge cases, but if the tradeof is between that specific sample vs any actual sample code extracted from existing software (which I'm doing with this lagre int optimization serie) I'd rather go for the one that is actually used.


https://reviews.llvm.org/D32925