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

[Eli Friedman via Phabricator via llvm-commits]

efriedma 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));
----------------
jgreenhalgh wrote:
> 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.
> What code is producing an ADDCARRY like this?

I meant, what code in the ARM backend is producing an ISD::ADDCARRY node with an illegal type?


https://reviews.llvm.org/D38942