Skip to content

Commit

Permalink
[CodeGen][SelectionDAG] Fix tiny bug in ExpandIntRes_UADDSUBO
Browse files Browse the repository at this point in the history
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.

Reviewers: bogner, lebedev.ri

Reviewed By: lebedev.ri

Subscribers: lebedev.ri, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68123
  • Loading branch information
ibookstein authored and LebedevRI committed Oct 25, 2019
1 parent 6df7ef0 commit 59a51d8
Showing 1 changed file with 22 additions and 9 deletions.
31 changes: 22 additions & 9 deletions llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
Expand Up @@ -2304,11 +2304,27 @@ void DAGTypeLegalizer::ExpandIntRes_UADDSUBO(SDNode *N,

SDValue Ovf;

bool HasOpCarry = TLI.isOperationLegalOrCustom(
N->getOpcode() == ISD::ADD ? ISD::ADDCARRY : ISD::SUBCARRY,
TLI.getTypeToExpandTo(*DAG.getContext(), LHS.getValueType()));
unsigned CarryOp, NoCarryOp;
ISD::CondCode Cond;
switch(N->getOpcode()) {
case ISD::UADDO:
CarryOp = ISD::ADDCARRY;
NoCarryOp = ISD::ADD;
Cond = ISD::SETULT;
break;
case ISD::USUBO:
CarryOp = ISD::SUBCARRY;
NoCarryOp = ISD::SUB;
Cond = ISD::SETUGT;
break;
default:
llvm_unreachable("Node has unexpected Opcode");
}

if (HasOpCarry) {
bool HasCarryOp = TLI.isOperationLegalOrCustom(
CarryOp, TLI.getTypeToExpandTo(*DAG.getContext(), LHS.getValueType()));

if (HasCarryOp) {
// Expand the subcomponents.
SDValue LHSL, LHSH, RHSL, RHSH;
GetExpandedInteger(LHS, LHSL, LHSH);
Expand All @@ -2317,22 +2333,19 @@ void DAGTypeLegalizer::ExpandIntRes_UADDSUBO(SDNode *N,
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);
Hi = DAG.getNode(CarryOp, 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);
SDValue Sum = DAG.getNode(NoCarryOp, 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);
}

Expand Down

0 comments on commit 59a51d8

Please sign in to comment.