[PATCH] D35192: [ARM] Use ADDCARRY / SUBCARRY

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 marked an inline comment as done.
rogfer01 added a comment.

Hi @efriedma, thanks for the review, please find the updated changes. I was wrongly handling `ISD::SUBCARRY` because its operand is a borrow while `ARMISD::SUBE` expects a carry.



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:4011
+        DAG.getNode(ARMISD::SUBE, dl, VTs, DAG.getConstant(1, dl, MVT::i32),
+                    DAG.getConstant(0, dl, MVT::i32), Carry);
+    break;
----------------
efriedma wrote:
> Oops, I should have spotted the issue here earlier.
> 
> The value produced by sbc is a+~b+C. So (ARMISD::SUBE i32 1, i32 0) is computing 1+-1+C==C. The value you actually need to produce here is 1-C.
> 
> In practice, this means we're generating wrong code for the usub_overflow testcase.
Thanks Eli, this was a serious bug. Sorry for that. I used the obvious approach instead.


https://reviews.llvm.org/D35192