[PATCH] D38942: [DAG] Promote ADDCARRY / SUBCARRY

[James Greenhalgh via Phabricator via llvm-commits]

jgreenhalgh added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:777-787
+  SDValue LHS = GetPromotedInteger(N->getOperand(0));
+  SDValue RHS = GetPromotedInteger(N->getOperand(1));
+
+  EVT ValueVTs[] = { LHS.getValueType(), N->getValueType(1) };
+
+  SDValue Res = DAG.getNode(N->getOpcode(), SDLoc(N),
+                     DAG.getVTList(ValueVTs), LHS, RHS, N->getOperand(2));
----------------
rogfer01 wrote:
> efriedma wrote:
> > rogfer01 wrote:
> > > This looks like a lot like the "dual" of `PromoteIntRes_Overflow` but replacing the users of `:1` (instead of `:0`). It generates correct code in Arm but I may be missing some subtlety here.
> > Yes, you're missing a very important piece here: the output carry bit is wrong.  You have to check whether the addition would overflow in the narrow type, not just whether it would overflow in the promoted type.
> > 
> > What code is producing an ADDCARRY like this?
> Ah! Of course!
> 
> Thanks a lot Eli. This will need fixing. 
> 
> A testcase (found by csmith)
> ```
> unsigned a, b, e;
> void fn1() {
>   long d = (a + b < b) - e;
>   short c = d + 1;
>   if (c)
>    for (;;)
>    ;
> }
> ```
I'm not sure this comment is correct. Because you're sign extending, you want to treat the narrow mode as a signed value, which means you only have 3 cases to consider:

1) Addition of two negative numbers - In two's compliment this will always set the carry flag. Sign extension would leave a 1 in the most significant bit for both widened input values, and so addition in the wider mode would also always set the carry flag.

2) Addition of two positive numbers - In two's compliment this can never set the carry flag in the narrow mode. Sign extension leave a 0 in the two most significant bits for both widened input values, and so addition in the wider mode would also never set the carry flag.

3) Addition of a positive number and a negative number - This will sometimes set the carry flag in the narrow mode. The insight here is that the negative number will propagate 1 to all widened bits when sign extended, without changing the bits of the two narrow values. If a carry would be generated from the narrow values, this would also propagate through the upper bits of the widened addition, and a carry would be generated. If no carry is generated from addition in the narrow mode, then no carry is generated in the wider mode.


https://reviews.llvm.org/D38942