Skip to content

Commit

Permalink
Revert r312898 "[ARM] Use ADDCARRY / SUBCARRY"
Browse files Browse the repository at this point in the history 
It caused PR34564.

> This is a preparatory step for D34515 and also is being recommitted as its
> first version caused PR34045.
>
> This change:
>  - makes nodes ISD::ADDCARRY and ISD::SUBCARRY legal for i32
>  - lowering is done by first converting the boolean value into the carry flag
>    using (_, C) ← (ARMISD::ADDC R, -1) and converted back to an integer value
>    using (R, _) ← (ARMISD::ADDE 0, 0, C). An ARMISD::ADDE between the two
>    operations does the actual addition.
>  - for subtraction, given that ISD::SUBCARRY second result is actually a
>    borrow, we need to invert the value of the second operand and result before
>    and after using ARMISD::SUBE. We need to invert the carry result of
>    ARMISD::SUBE to preserve the semantics.
>  - given that the generic combiner may lower ISD::ADDCARRY and
>    ISD::SUBCARRYinto ISD::UADDO and ISD::USUBO we need to update their lowering
>    as well otherwise i64 operations now would require branches. This implies
>    updating the corresponding test for unsigned.
>  - add new combiner to remove the redundant conversions from/to carry flags
>    to/from boolean values (ARMISD::ADDC (ARMISD::ADDE 0, 0, C), -1) → C
>  - fixes PR34045
>
> Differential Revision: https://reviews.llvm.org/D35192

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@312980 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@zmodem
zmodem committed Sep 11, 2017
1 parent 9e8c499 commit e4fcd18
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 294 deletions.
185 changes: 19 additions & 166 deletions lib/Target/ARM/ARMISelLowering.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -802,9 +802,6 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
setOperationAction(ISD::SSUBO, MVT::i32, Custom);
setOperationAction(ISD::USUBO, MVT::i32, Custom);

setOperationAction(ISD::ADDCARRY, MVT::i32, Custom);
setOperationAction(ISD::SUBCARRY, MVT::i32, Custom);

// i64 operation support.
setOperationAction(ISD::MUL, MVT::i64, Expand);
setOperationAction(ISD::MULHU, MVT::i32, Expand);
Expand Down Expand Up @@ -3956,7 +3953,7 @@ ARMTargetLowering::getARMXALUOOp(SDValue Op, SelectionDAG &DAG,
}

SDValue
ARMTargetLowering::LowerSignedALUO(SDValue Op, SelectionDAG &DAG) const {
ARMTargetLowering::LowerXALUO(SDValue Op, SelectionDAG &DAG) const {
// Let legalize expand this if it isn't a legal type yet.
if (!DAG.getTargetLoweringInfo().isTypeLegal(Op.getValueType()))
return SDValue();
Expand All @@ -3978,66 +3975,6 @@ ARMTargetLowering::LowerSignedALUO(SDValue Op, SelectionDAG &DAG) const {
return DAG.getNode(ISD::MERGE_VALUES, dl, VTs, Value, Overflow);
}

static SDValue ConvertBooleanCarryToCarryFlag(SDValue BoolCarry,
SelectionDAG &DAG) {
SDLoc DL(BoolCarry);
EVT CarryVT = BoolCarry.getValueType();

APInt NegOne = APInt::getAllOnesValue(CarryVT.getScalarSizeInBits());
// This converts the boolean value carry into the carry flag by doing
// ARMISD::ADDC Carry, ~0
return DAG.getNode(ARMISD::ADDC, DL, DAG.getVTList(CarryVT, MVT::i32),
BoolCarry, DAG.getConstant(NegOne, DL, CarryVT));
}

static SDValue ConvertCarryFlagToBooleanCarry(SDValue Flags, EVT VT,
SelectionDAG &DAG) {
SDLoc DL(Flags);

// Now convert the carry flag into a boolean carry. We do this
// using ARMISD:ADDE 0, 0, Carry
return DAG.getNode(ARMISD::ADDE, DL, DAG.getVTList(VT, MVT::i32),
DAG.getConstant(0, DL, MVT::i32),
DAG.getConstant(0, DL, MVT::i32), Flags);
}

SDValue ARMTargetLowering::LowerUnsignedALUO(SDValue Op,
SelectionDAG &DAG) const {
// Let legalize expand this if it isn't a legal type yet.
if (!DAG.getTargetLoweringInfo().isTypeLegal(Op.getValueType()))
return SDValue();

SDValue LHS = Op.getOperand(0);
SDValue RHS = Op.getOperand(1);
SDLoc dl(Op);

EVT VT = Op.getValueType();
SDVTList VTs = DAG.getVTList(VT, MVT::i32);
SDValue Value;
SDValue Overflow;
switch (Op.getOpcode()) {
default:
llvm_unreachable("Unknown overflow instruction!");
case ISD::UADDO:
Value = DAG.getNode(ARMISD::ADDC, dl, VTs, LHS, RHS);
// Convert the carry flag into a boolean value.
Overflow = ConvertCarryFlagToBooleanCarry(Value.getValue(1), VT, DAG);
break;
case ISD::USUBO: {
Value = DAG.getNode(ARMISD::SUBC, dl, VTs, LHS, RHS);
// Convert the carry flag into a boolean value.
Overflow = ConvertCarryFlagToBooleanCarry(Value.getValue(1), VT, DAG);
// ARMISD::SUBC returns 0 when we have to borrow, so make it an overflow
// value. So compute 1 - C.
Overflow = DAG.getNode(ISD::SUB, dl, VTs,
DAG.getConstant(1, dl, MVT::i32), Overflow);
break;
}
}

return DAG.getNode(ISD::MERGE_VALUES, dl, VTs, Value, Overflow);
}

SDValue ARMTargetLowering::LowerSELECT(SDValue Op, SelectionDAG &DAG) const {
SDValue Cond = Op.getOperand(0);
SDValue SelectTrue = Op.getOperand(1);
Expand Down Expand Up @@ -7443,53 +7380,6 @@ static SDValue LowerADDC_ADDE_SUBC_SUBE(SDValue Op, SelectionDAG &DAG) {
Op.getOperand(1), Op.getOperand(2));
}

static SDValue LowerADDSUBCARRY(SDValue Op, SelectionDAG &DAG) {
SDNode *N = Op.getNode();
EVT VT = N->getValueType(0);
SDVTList VTs = DAG.getVTList(VT, MVT::i32);

SDValue Carry = Op.getOperand(2);
EVT CarryVT = Carry.getValueType();

SDLoc DL(Op);

APInt NegOne = APInt::getAllOnesValue(CarryVT.getScalarSizeInBits());

SDValue Result;
if (Op.getOpcode() == ISD::ADDCARRY) {
// This converts the boolean value carry into the carry flag.
Carry = ConvertBooleanCarryToCarryFlag(Carry, DAG);

// Do the addition proper using the carry flag we wanted.
Result = DAG.getNode(ARMISD::ADDE, DL, VTs, Op.getOperand(0),
Op.getOperand(1), Carry.getValue(1));

// Now convert the carry flag into a boolean value.
Carry = ConvertCarryFlagToBooleanCarry(Result.getValue(1), VT, DAG);
} else {
// ARMISD::SUBE expects a carry not a borrow like ISD::SUBCARRY so we
// have to invert the carry first.
Carry =
DAG.getNode(ISD::SUB, DL, VTs, DAG.getConstant(1, DL, MVT::i32), Carry);
// This converts the boolean value carry into the carry flag.
Carry = ConvertBooleanCarryToCarryFlag(Carry, DAG);

// Do the subtraction proper using the carry flag we wanted.
Result = DAG.getNode(ARMISD::SUBE, DL, VTs, Op.getOperand(0),
Op.getOperand(1), Carry.getValue(1));

// Now convert the carry flag into a boolean value.
Carry = ConvertCarryFlagToBooleanCarry(Result.getValue(1), VT, DAG);
// But the carry returned by ARMISD::SUBE is not a borrow as expected
// by ISD::SUBCARRY, so compute 1 - C.
Carry =
DAG.getNode(ISD::SUB, DL, VTs, DAG.getConstant(1, DL, MVT::i32), Carry);
}

// Return both values.
return DAG.getNode(ISD::MERGE_VALUES, DL, N->getVTList(), Result, Carry);
}

SDValue ARMTargetLowering::LowerFSINCOS(SDValue Op, SelectionDAG &DAG) const {
assert(Subtarget->isTargetDarwin());

Expand Down Expand Up @@ -7844,14 +7734,11 @@ SDValue ARMTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
case ISD::ADDE:
case ISD::SUBC:
case ISD::SUBE: return LowerADDC_ADDE_SUBC_SUBE(Op, DAG);
case ISD::ADDCARRY:
case ISD::SUBCARRY: return LowerADDSUBCARRY(Op, DAG);
case ISD::SADDO:
case ISD::SSUBO:
return LowerSignedALUO(Op, DAG);
case ISD::UADDO:
case ISD::SSUBO:
case ISD::USUBO:
return LowerUnsignedALUO(Op, DAG);
return LowerXALUO(Op, DAG);
case ISD::ATOMIC_LOAD:
case ISD::ATOMIC_STORE: return LowerAtomicLoadStore(Op, DAG);
case ISD::FSINCOS: return LowerFSINCOS(Op, DAG);
Expand Down Expand Up @@ -9800,19 +9687,19 @@ static SDValue AddCombineTo64bitMLAL(SDNode *AddeNode,
// a S/UMLAL instruction.
// UMUL_LOHI
// / :lo \ :hi
// V \ [no multiline comment]
// loAdd -> ADDC |
// \ :carry /
// V V
// ADDE <- hiAdd
// / \ [no multiline comment]
// loAdd -> ADDE |
// \ :glue /
// \ /
// ADDC <- hiAdd
//
assert(AddeNode->getOpcode() == ARMISD::ADDE && "Expect an ADDE");

assert(AddeNode->getNumOperands() == 3 &&
AddeNode->getOperand(2).getValueType() == MVT::i32 &&
"ADDE node has the wrong inputs");

// Check that we are chained to the right ADDC node.
// Check that we have a glued ADDC node.
SDNode* AddcNode = AddeNode->getOperand(2).getNode();
if (AddcNode->getOpcode() != ARMISD::ADDC)
return SDValue();
Expand Down Expand Up @@ -9863,7 +9750,7 @@ static SDValue AddCombineTo64bitMLAL(SDNode *AddeNode,
SDValue* LoMul = nullptr;
SDValue* LowAdd = nullptr;

// Ensure that ADDE is from high result of ISD::xMUL_LOHI.
// Ensure that ADDE is from high result of ISD::SMUL_LOHI.
if ((AddeOp0 != MULOp.getValue(1)) && (AddeOp1 != MULOp.getValue(1)))
return SDValue();

Expand All @@ -9888,11 +9775,6 @@ static SDValue AddCombineTo64bitMLAL(SDNode *AddeNode,
if (!LoMul)
return SDValue();

// If HiAdd is a predecessor of ADDC, the replacement below will create a
// cycle.
if (AddcNode->isPredecessorOf(HiAdd->getNode()))
return SDValue();

// Create the merged node.
SelectionDAG &DAG = DCI.DAG;

Expand Down Expand Up @@ -9995,22 +9877,8 @@ static SDValue PerformUMLALCombine(SDNode *N, SelectionDAG &DAG,
return SDValue();
}

static SDValue PerformAddcSubcCombine(SDNode *N,
TargetLowering::DAGCombinerInfo &DCI,
static SDValue PerformAddcSubcCombine(SDNode *N, SelectionDAG &DAG,
const ARMSubtarget *Subtarget) {
SelectionDAG &DAG(DCI.DAG);

if (N->getOpcode() == ARMISD::ADDC) {
// (ADDC (ADDE 0, 0, C), -1) -> C
SDValue LHS = N->getOperand(0);
SDValue RHS = N->getOperand(1);
if (LHS->getOpcode() == ARMISD::ADDE &&
isNullConstant(LHS->getOperand(0)) &&
isNullConstant(LHS->getOperand(1)) && isAllOnesConstant(RHS)) {
return DCI.CombineTo(N, SDValue(N, 0), LHS->getOperand(2));
}
}

if (Subtarget->isThumb1Only()) {
SDValue RHS = N->getOperand(1);
if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(RHS)) {
Expand Down Expand Up @@ -11899,14 +11767,6 @@ static SDValue PerformExtendCombine(SDNode *N, SelectionDAG &DAG,
return SDValue();
}

static const APInt *isPowerOf2Constant(SDValue V) {
ConstantSDNode *C = dyn_cast<ConstantSDNode>(V);
if (!C)
return nullptr;
const APInt *CV = &C->getAPIntValue();
return CV->isPowerOf2() ? CV : nullptr;
}

SDValue ARMTargetLowering::PerformCMOVToBFICombine(SDNode *CMOV, SelectionDAG &DAG) const {
// If we have a CMOV, OR and AND combination such as:
// if (x & CN)
Expand Down Expand Up @@ -11935,8 +11795,8 @@ SDValue ARMTargetLowering::PerformCMOVToBFICombine(SDNode *CMOV, SelectionDAG &D
SDValue And = CmpZ->getOperand(0);
if (And->getOpcode() != ISD::AND)
return SDValue();
const APInt *AndC = isPowerOf2Constant(And->getOperand(1));
if (!AndC)
ConstantSDNode *AndC = dyn_cast<ConstantSDNode>(And->getOperand(1));
if (!AndC || !AndC->getAPIntValue().isPowerOf2())
return SDValue();
SDValue X = And->getOperand(0);

Expand Down Expand Up @@ -11976,7 +11836,7 @@ SDValue ARMTargetLowering::PerformCMOVToBFICombine(SDNode *CMOV, SelectionDAG &D
SDValue V = Y;
SDLoc dl(X);
EVT VT = X.getValueType();
unsigned BitInX = AndC->logBase2();
unsigned BitInX = AndC->getAPIntValue().logBase2();

if (BitInX != 0) {
// We must shift X first.
Expand Down Expand Up @@ -12137,7 +11997,7 @@ SDValue ARMTargetLowering::PerformDAGCombine(SDNode *N,
case ISD::XOR: return PerformXORCombine(N, DCI, Subtarget);
case ISD::AND: return PerformANDCombine(N, DCI, Subtarget);
case ARMISD::ADDC:
case ARMISD::SUBC: return PerformAddcSubcCombine(N, DCI, Subtarget);
case ARMISD::SUBC: return PerformAddcSubcCombine(N, DCI.DAG, Subtarget);
case ARMISD::SUBE: return PerformAddeSubeCombine(N, DCI.DAG, Subtarget);
case ARMISD::BFI: return PerformBFICombine(N, DCI);
case ARMISD::VMOVRRD: return PerformVMOVRRDCombine(N, DCI, Subtarget);
Expand Down Expand Up @@ -12833,17 +12693,10 @@ void ARMTargetLowering::computeKnownBitsForTargetNode(const SDValue Op,
case ARMISD::ADDE:
case ARMISD::SUBC:
case ARMISD::SUBE:
// Special cases when we convert a carry to a boolean.
if (Op.getResNo() == 0) {
SDValue LHS = Op.getOperand(0);
SDValue RHS = Op.getOperand(1);
// (ADDE 0, 0, C) will give us a single bit.
if (Op->getOpcode() == ARMISD::ADDE && isNullConstant(LHS) &&
isNullConstant(RHS)) {
Known.Zero |= APInt::getHighBitsSet(BitWidth, BitWidth - 1);
return;
}
}
// These nodes' second result is a boolean
if (Op.getResNo() == 0)
break;
Known.Zero |= APInt::getHighBitsSet(BitWidth, BitWidth - 1);
break;
case ARMISD::CMOV: {
// Bits are known zero/one if known on the LHS and RHS.
Expand Down
3 changes: 1 addition & 2 deletions lib/Target/ARM/ARMISelLowering.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,7 @@ class InstrItineraryData;
SDValue LowerGlobalTLSAddressWindows(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerGLOBAL_OFFSET_TABLE(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerBR_JT(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerSignedALUO(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerUnsignedALUO(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerXALUO(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerSELECT(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerBR_CC(SDValue Op, SelectionDAG &DAG) const;
Expand Down
Loading

0 comments on commit e4fcd18

Please sign in to comment.