Skip to content

Commit 76732ac

Browse files
author
Eli Friedman
committed
[ARM] Revert r297443 and r297820.
The glueless lowering of addc/adde in Thumb1 has known serious miscompiles (see https://reviews.llvm.org/D31081), and r297820 causes an infinite loop for certain constructs. It's not clear when they will be fixed, so let's just take them out of the tree for now. (I resolved a small conflict with r297453.) llvm-svn: 298328
1 parent 7668182 commit 76732ac

File tree

4 files changed

+39
-287
lines changed

4 files changed

+39
-287
lines changed

‎llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2028,16 +2028,6 @@ static const AddSubFlagsOpcodePair AddSubFlagsOpcodeMap[] = {
20282028
{ARM::RSBSrsi, ARM::RSBrsi},
20292029
{ARM::RSBSrsr, ARM::RSBrsr},
20302030

2031-
{ARM::tADDSi3, ARM::tADDi3},
2032-
{ARM::tADDSi8, ARM::tADDi8},
2033-
{ARM::tADDSrr, ARM::tADDrr},
2034-
{ARM::tADCS, ARM::tADC},
2035-
2036-
{ARM::tSUBSi3, ARM::tSUBi3},
2037-
{ARM::tSUBSi8, ARM::tSUBi8},
2038-
{ARM::tSUBSrr, ARM::tSUBrr},
2039-
{ARM::tSBCS, ARM::tSBC},
2040-
20412031
{ARM::t2ADDSri, ARM::t2ADDri},
20422032
{ARM::t2ADDSrr, ARM::t2ADDrr},
20432033
{ARM::t2ADDSrs, ARM::t2ADDrs},

‎llvm/lib/Target/ARM/ARMISelLowering.cpp

Lines changed: 13 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -822,10 +822,13 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
822822
setOperationAction(ISD::SRL, MVT::i64, Custom);
823823
setOperationAction(ISD::SRA, MVT::i64, Custom);
824824

825-
setOperationAction(ISD::ADDC, MVT::i32, Custom);
826-
setOperationAction(ISD::ADDE, MVT::i32, Custom);
827-
setOperationAction(ISD::SUBC, MVT::i32, Custom);
828-
setOperationAction(ISD::SUBE, MVT::i32, Custom);
825+
if (!Subtarget->isThumb1Only()) {
826+
// FIXME: We should do this for Thumb1 as well.
827+
setOperationAction(ISD::ADDC, MVT::i32, Custom);
828+
setOperationAction(ISD::ADDE, MVT::i32, Custom);
829+
setOperationAction(ISD::SUBC, MVT::i32, Custom);
830+
setOperationAction(ISD::SUBE, MVT::i32, Custom);
831+
}
829832

830833
if (!Subtarget->isThumb1Only() && Subtarget->hasV6T2Ops())
831834
setOperationAction(ISD::BITREVERSE, MVT::i32, Legal);
@@ -9093,45 +9096,19 @@ void ARMTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
90939096

90949097
// Rename pseudo opcodes.
90959098
unsigned NewOpc = convertAddSubFlagsOpcode(MI.getOpcode());
9096-
unsigned ccOutIdx;
90979099
if (NewOpc) {
90989100
const ARMBaseInstrInfo *TII = Subtarget->getInstrInfo();
90999101
MCID = &TII->get(NewOpc);
91009102

9101-
assert(MCID->getNumOperands() ==
9102-
MI.getDesc().getNumOperands() + 5 - MI.getDesc().getSize()
9103-
&& "converted opcode should be the same except for cc_out"
9104-
" (and, on Thumb1, pred)");
9103+
assert(MCID->getNumOperands() == MI.getDesc().getNumOperands() + 1 &&
9104+
"converted opcode should be the same except for cc_out");
91059105

91069106
MI.setDesc(*MCID);
91079107

91089108
// Add the optional cc_out operand
91099109
MI.addOperand(MachineOperand::CreateReg(0, /*isDef=*/true));
9110-
9111-
// On Thumb1, move all input operands to the end, then add the predicate
9112-
if (Subtarget->isThumb1Only()) {
9113-
for (unsigned c = MCID->getNumOperands() - 4; c--;) {
9114-
MI.addOperand(MI.getOperand(1));
9115-
MI.RemoveOperand(1);
9116-
}
9117-
9118-
// Restore the ties
9119-
for (unsigned i = MI.getNumOperands(); i--;) {
9120-
const MachineOperand& op = MI.getOperand(i);
9121-
if (op.isReg() && op.isUse()) {
9122-
int DefIdx = MCID->getOperandConstraint(i, MCOI::TIED_TO);
9123-
if (DefIdx != -1)
9124-
MI.tieOperands(DefIdx, i);
9125-
}
9126-
}
9127-
9128-
MI.addOperand(MachineOperand::CreateImm(ARMCC::AL));
9129-
MI.addOperand(MachineOperand::CreateReg(0, /*isDef=*/false));
9130-
ccOutIdx = 1;
9131-
} else
9132-
ccOutIdx = MCID->getNumOperands() - 1;
9133-
} else
9134-
ccOutIdx = MCID->getNumOperands() - 1;
9110+
}
9111+
unsigned ccOutIdx = MCID->getNumOperands() - 1;
91359112

91369113
// Any ARM instruction that sets the 's' bit should specify an optional
91379114
// "cc_out" operand in the last operand position.
@@ -9162,9 +9139,7 @@ void ARMTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
91629139
if (deadCPSR) {
91639140
assert(!MI.getOperand(ccOutIdx).getReg() &&
91649141
"expect uninitialized optional cc_out operand");
9165-
// Thumb1 instructions must have the S bit even if the CPSR is dead.
9166-
if (!Subtarget->isThumb1Only())
9167-
return;
9142+
return;
91689143
}
91699144

91709145
// If this instruction was defined with an optional CPSR def and its dag node
@@ -9784,48 +9759,6 @@ static SDValue PerformUMLALCombine(SDNode *N, SelectionDAG &DAG,
97849759
return SDValue();
97859760
}
97869761

9787-
static SDValue PerformAddcSubcCombine(SDNode *N, SelectionDAG &DAG,
9788-
const ARMSubtarget *Subtarget) {
9789-
if (Subtarget->isThumb1Only()) {
9790-
SDValue RHS = N->getOperand(1);
9791-
if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(RHS)) {
9792-
int64_t imm = C->getSExtValue();
9793-
if (imm < 0) {
9794-
SDLoc DL(N);
9795-
RHS = DAG.getConstant(-imm, DL, MVT::i32);
9796-
unsigned Opcode = (N->getOpcode() == ARMISD::ADDC) ? ARMISD::SUBC
9797-
: ARMISD::ADDC;
9798-
return DAG.getNode(Opcode, DL, N->getVTList(), N->getOperand(0), RHS);
9799-
}
9800-
}
9801-
}
9802-
return SDValue();
9803-
}
9804-
9805-
static SDValue PerformAddeSubeCombine(SDNode *N, SelectionDAG &DAG,
9806-
const ARMSubtarget *Subtarget) {
9807-
if (Subtarget->isThumb1Only()) {
9808-
SDValue RHS = N->getOperand(1);
9809-
if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(RHS)) {
9810-
int64_t imm = C->getSExtValue();
9811-
if (imm < 0) {
9812-
SDLoc DL(N);
9813-
9814-
// The with-carry-in form matches bitwise not instead of the negation.
9815-
// Effectively, the inverse interpretation of the carry flag already
9816-
// accounts for part of the negation.
9817-
RHS = DAG.getConstant(~imm, DL, MVT::i32);
9818-
9819-
unsigned Opcode = (N->getOpcode() == ARMISD::ADDE) ? ARMISD::SUBE
9820-
: ARMISD::ADDE;
9821-
return DAG.getNode(Opcode, DL, N->getVTList(),
9822-
N->getOperand(0), RHS, N->getOperand(2));
9823-
}
9824-
}
9825-
}
9826-
return SDValue();
9827-
}
9828-
98299762
/// PerformADDECombine - Target-specific dag combine transform from
98309763
/// ARMISD::ADDC, ARMISD::ADDE, and ISD::MUL_LOHI to MLAL or
98319764
/// ARMISD::ADDC, ARMISD::ADDE and ARMISD::UMLAL to ARMISD::UMAAL
@@ -9834,7 +9767,7 @@ static SDValue PerformADDECombine(SDNode *N,
98349767
const ARMSubtarget *Subtarget) {
98359768
// Only ARM and Thumb2 support UMLAL/SMLAL.
98369769
if (Subtarget->isThumb1Only())
9837-
return PerformAddeSubeCombine(N, DCI.DAG, Subtarget);
9770+
return SDValue();
98389771

98399772
// Only perform the checks after legalize when the pattern is available.
98409773
if (DCI.isBeforeLegalize()) return SDValue();
@@ -11934,9 +11867,6 @@ SDValue ARMTargetLowering::PerformDAGCombine(SDNode *N,
1193411867
case ISD::OR: return PerformORCombine(N, DCI, Subtarget);
1193511868
case ISD::XOR: return PerformXORCombine(N, DCI, Subtarget);
1193611869
case ISD::AND: return PerformANDCombine(N, DCI, Subtarget);
11937-
case ARMISD::ADDC:
11938-
case ARMISD::SUBC: return PerformAddcSubcCombine(N, DCI.DAG, Subtarget);
11939-
case ARMISD::SUBE: return PerformAddeSubeCombine(N, DCI.DAG, Subtarget);
1194011870
case ARMISD::BFI: return PerformBFICombine(N, DCI);
1194111871
case ARMISD::VMOVRRD: return PerformVMOVRRDCombine(N, DCI, Subtarget);
1194211872
case ARMISD::VMOVDRR: return PerformVMOVDRRCombine(N, DCI.DAG);

‎llvm/lib/Target/ARM/ARMInstrThumb.td

Lines changed: 18 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,7 @@ let isAdd = 1 in {
905905
def tADC : // A8.6.2
906906
T1sItDPEncode<0b0101, (outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm), IIC_iALUr,
907907
"adc", "\t$Rdn, $Rm",
908-
[]>, Sched<[WriteALU]>;
908+
[(set tGPR:$Rdn, (adde tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>;
909909

910910
// Add immediate
911911
def tADDi3 : // A8.6.4 T1
@@ -933,43 +933,6 @@ let isAdd = 1 in {
933933
"add", "\t$Rd, $Rn, $Rm",
934934
[(set tGPR:$Rd, (add tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>;
935935

936-
/// Similar to the above except these set the 's' bit so the
937-
/// instruction modifies the CPSR register.
938-
///
939-
/// These opcodes will be converted to the real non-S opcodes by
940-
/// AdjustInstrPostInstrSelection after giving then an optional CPSR operand.
941-
let hasPostISelHook = 1, Defs = [CPSR] in {
942-
let isCommutable = 1 in
943-
def tADCS : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm),
944-
2, IIC_iALUr,
945-
[(set tGPR:$Rdn, CPSR, (ARMadde tGPR:$Rn, tGPR:$Rm,
946-
CPSR))]>,
947-
Requires<[IsThumb1Only]>,
948-
Sched<[WriteALU]>;
949-
950-
def tADDSi3 : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rm, imm0_7:$imm3),
951-
2, IIC_iALUi,
952-
[(set tGPR:$Rd, CPSR, (ARMaddc tGPR:$Rm,
953-
imm0_7:$imm3))]>,
954-
Requires<[IsThumb1Only]>,
955-
Sched<[WriteALU]>;
956-
957-
def tADDSi8 : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, imm0_255:$imm8),
958-
2, IIC_iALUi,
959-
[(set tGPR:$Rdn, CPSR, (ARMaddc tGPR:$Rn,
960-
imm8_255:$imm8))]>,
961-
Requires<[IsThumb1Only]>,
962-
Sched<[WriteALU]>;
963-
964-
let isCommutable = 1 in
965-
def tADDSrr : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rn, tGPR:$Rm),
966-
2, IIC_iALUr,
967-
[(set tGPR:$Rd, CPSR, (ARMaddc tGPR:$Rn,
968-
tGPR:$Rm))]>,
969-
Requires<[IsThumb1Only]>,
970-
Sched<[WriteALU]>;
971-
}
972-
973936
let hasSideEffects = 0 in
974937
def tADDhirr : T1pIt<(outs GPR:$Rdn), (ins GPR:$Rn, GPR:$Rm), IIC_iALUr,
975938
"add", "\t$Rdn, $Rm", []>,
@@ -1229,7 +1192,7 @@ def tSBC : // A8.6.151
12291192
T1sItDPEncode<0b0110, (outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm),
12301193
IIC_iALUr,
12311194
"sbc", "\t$Rdn, $Rm",
1232-
[]>,
1195+
[(set tGPR:$Rdn, (sube tGPR:$Rn, tGPR:$Rm))]>,
12331196
Sched<[WriteALU]>;
12341197

12351198
// Subtract immediate
@@ -1258,41 +1221,6 @@ def tSUBrr : // A8.6.212
12581221
[(set tGPR:$Rd, (sub tGPR:$Rn, tGPR:$Rm))]>,
12591222
Sched<[WriteALU]>;
12601223

1261-
/// Similar to the above except these set the 's' bit so the
1262-
/// instruction modifies the CPSR register.
1263-
///
1264-
/// These opcodes will be converted to the real non-S opcodes by
1265-
/// AdjustInstrPostInstrSelection after giving then an optional CPSR operand.
1266-
let hasPostISelHook = 1, Defs = [CPSR] in {
1267-
def tSBCS : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm),
1268-
2, IIC_iALUr,
1269-
[(set tGPR:$Rdn, CPSR, (ARMsube tGPR:$Rn, tGPR:$Rm,
1270-
CPSR))]>,
1271-
Requires<[IsThumb1Only]>,
1272-
Sched<[WriteALU]>;
1273-
1274-
def tSUBSi3 : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rm, imm0_7:$imm3),
1275-
2, IIC_iALUi,
1276-
[(set tGPR:$Rd, CPSR, (ARMsubc tGPR:$Rm,
1277-
imm0_7:$imm3))]>,
1278-
Requires<[IsThumb1Only]>,
1279-
Sched<[WriteALU]>;
1280-
1281-
def tSUBSi8 : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, imm0_255:$imm8),
1282-
2, IIC_iALUi,
1283-
[(set tGPR:$Rdn, CPSR, (ARMsubc tGPR:$Rn,
1284-
imm8_255:$imm8))]>,
1285-
Requires<[IsThumb1Only]>,
1286-
Sched<[WriteALU]>;
1287-
1288-
def tSUBSrr : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rn, tGPR:$Rm),
1289-
2, IIC_iALUr,
1290-
[(set tGPR:$Rd, CPSR, (ARMsubc tGPR:$Rn,
1291-
tGPR:$Rm))]>,
1292-
Requires<[IsThumb1Only]>,
1293-
Sched<[WriteALU]>;
1294-
}
1295-
12961224
// Sign-extend byte
12971225
def tSXTB : // A8.6.222
12981226
T1pIMiscEncode<{0,0,1,0,0,1,?}, (outs tGPR:$Rd), (ins tGPR:$Rm),
@@ -1453,6 +1381,22 @@ def : T1Pat<(ARMcmpZ tGPR:$Rn, imm0_255:$imm8),
14531381
def : T1Pat<(ARMcmpZ tGPR:$Rn, tGPR:$Rm),
14541382
(tCMPr tGPR:$Rn, tGPR:$Rm)>;
14551383

1384+
// Add with carry
1385+
def : T1Pat<(addc tGPR:$lhs, imm0_7:$rhs),
1386+
(tADDi3 tGPR:$lhs, imm0_7:$rhs)>;
1387+
def : T1Pat<(addc tGPR:$lhs, imm8_255:$rhs),
1388+
(tADDi8 tGPR:$lhs, imm8_255:$rhs)>;
1389+
def : T1Pat<(addc tGPR:$lhs, tGPR:$rhs),
1390+
(tADDrr tGPR:$lhs, tGPR:$rhs)>;
1391+
1392+
// Subtract with carry
1393+
def : T1Pat<(addc tGPR:$lhs, imm0_7_neg:$rhs),
1394+
(tSUBi3 tGPR:$lhs, imm0_7_neg:$rhs)>;
1395+
def : T1Pat<(addc tGPR:$lhs, imm8_255_neg:$rhs),
1396+
(tSUBi8 tGPR:$lhs, imm8_255_neg:$rhs)>;
1397+
def : T1Pat<(subc tGPR:$lhs, tGPR:$rhs),
1398+
(tSUBrr tGPR:$lhs, tGPR:$rhs)>;
1399+
14561400
// Bswap 16 with load/store
14571401
def : T1Pat<(srl (bswap (extloadi16 t_addrmode_is2:$addr)), (i32 16)),
14581402
(tREV16 (tLDRHi t_addrmode_is2:$addr))>;

0 commit comments

Comments
 (0)