[CodeGen][SelectionDAG] Fix tiny bug in ExpandIntRes_UADDSUBO

[ibookstein]

Summary

Ternary expression checks for ISD::ADD instead of ISD::UADDO inside DAGTypeLegalizer::ExpandIntRes_UADDSUBO.
This means the ternary expression will evaluate to ISD::SUBCARRY for both ISD::UADDO and ISD::USUBO nodes.
Targets are likely to implement both, so impact will be very limited in practice.

[ibookstein]

Changed diff to be -U999999

[lebedev.ri]

Test missing.

[ibookstein]

As this would only affect a hypothetical target that supports ISD::SUBCARRY but not ISD::ADDCARRY, testing this via IR would seem overkill.
I think the correct level of testing for this would be UT for DAGTypeLegalizer, but the closest thing I could find was some of the things under llvm/unittests/CodeGen, like AArch64SelectionDAGTest.cpp.
Does that sound like it's in the right direction?

[lebedev.ri]

Two remarks:

• Code duplication
• If this is purely theoretical, can the dead code be simply dropped?

llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

2261

this

2273

this

2282

and this do basically the same thing.
At the very least you want to precompute which carry and non-carry opcodes you'll want to use, thus avoiding the bug altogether.

[ibookstein]

Is this along the lines of what you suggested?

[lebedev.ri]

That'll do, thanks!

[ibookstein]

I failed to mention during the review that I don't have commit access, so could anyone commit this on my behalf?

[ibookstein]

Ping
Is there anything else I should/could to to land this?

[lebedev.ri]

Right.
Now that we have migrated to git, can you paste your Author: name <[e-mail]> that should be used for the patch?

[ibookstein]

Yeah sure, thanks!
Author: Itay Bookstein <[e-mail]>

llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

void DAGTypeLegalizer::ExpandIntRes_UADDSUBO(SDNode *N,
                                              SDValue &Lo, SDValue &Hi) {
   SDValue LHS = N->getOperand(0);
   SDValue RHS = N->getOperand(1);
   SDLoc dl(N);
 
   SDValue Ovf;
 
   bool HasOpCarry = TLI.isOperationLegalOrCustom(
-      N->getOpcode() == ISD::ADD ? ISD::ADDCARRY : ISD::SUBCARRY,
+      N->getOpcode() == ISD::UADDO ? ISD::ADDCARRY : ISD::SUBCARRY,
       TLI.getTypeToExpandTo(*DAG.getContext(), LHS.getValueType()));
 
   if (HasOpCarry) {
     // Expand the subcomponents.
     SDValue LHSL, LHSH, RHSL, RHSH;
     GetExpandedInteger(LHS, LHSL, LHSH);
     GetExpandedInteger(RHS, RHSL, RHSH);
     SDVTList VTList = DAG.getVTList(LHSL.getValueType(), N->getValueType(1));
     SDValue LoOps[2] = { LHSL, RHSL };
     SDValue HiOps[3] = { LHSH, RHSH };
 
     unsigned Opc = N->getOpcode() == ISD::UADDO ? ISD::ADDCARRY : ISD::SUBCARRY;
     Lo = DAG.getNode(N->getOpcode(), dl, VTList, LoOps);
     HiOps[2] = Lo.getValue(1);
     Hi = DAG.getNode(Opc, dl, VTList, HiOps);
 
     Ovf = Hi.getValue(1);
   } else {
     // Expand the result by simply replacing it with the equivalent
     // non-overflow-checking operation.
     auto Opc = N->getOpcode() == ISD::UADDO ? ISD::ADD : ISD::SUB;
     SDValue Sum = DAG.getNode(Opc, dl, LHS.getValueType(), LHS, RHS);
     SplitInteger(Sum, Lo, Hi);
 
     // Calculate the overflow: addition overflows iff a + b < a, and subtraction
     // overflows iff a - b > a.
     auto Cond = N->getOpcode() == ISD::UADDO ? ISD::SETULT : ISD::SETUGT;
     Ovf = DAG.getSetCC(dl, N->getValueType(1), Sum, LHS, Cond);
   }