From e69c137f90e0fde49ecc489d57e461255f8b7c32 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 22 Mar 2017 05:07:44 +0000 Subject: [PATCH] Revert "[ARM] Recommit the glueless lowering of addc/adde in Thumb1, including the amended (no UB anymore) fix for adding/subtracting -2147483648." Fails check-llvm with ubsan This reverts commit r298417. llvm-svn: 298482 --- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp | 10 --- llvm/lib/Target/ARM/ARMISelLowering.cpp | 96 +++------------------ llvm/lib/Target/ARM/ARMInstrThumb.td | 92 ++++---------------- llvm/test/CodeGen/Thumb/long.ll | 140 ++----------------------------- 4 files changed, 39 insertions(+), 299 deletions(-) diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp index 4f5711c..7286882 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp @@ -2028,16 +2028,6 @@ static const AddSubFlagsOpcodePair AddSubFlagsOpcodeMap[] = { {ARM::RSBSrsi, ARM::RSBrsi}, {ARM::RSBSrsr, ARM::RSBrsr}, - {ARM::tADDSi3, ARM::tADDi3}, - {ARM::tADDSi8, ARM::tADDi8}, - {ARM::tADDSrr, ARM::tADDrr}, - {ARM::tADCS, ARM::tADC}, - - {ARM::tSUBSi3, ARM::tSUBi3}, - {ARM::tSUBSi8, ARM::tSUBi8}, - {ARM::tSUBSrr, ARM::tSUBrr}, - {ARM::tSBCS, ARM::tSBC}, - {ARM::t2ADDSri, ARM::t2ADDri}, {ARM::t2ADDSrr, ARM::t2ADDrr}, {ARM::t2ADDSrs, ARM::t2ADDrs}, diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp index 24ddb30..e372378 100644 --- a/llvm/lib/Target/ARM/ARMISelLowering.cpp +++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp @@ -822,10 +822,13 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM, setOperationAction(ISD::SRL, MVT::i64, Custom); setOperationAction(ISD::SRA, MVT::i64, Custom); - setOperationAction(ISD::ADDC, MVT::i32, Custom); - setOperationAction(ISD::ADDE, MVT::i32, Custom); - setOperationAction(ISD::SUBC, MVT::i32, Custom); - setOperationAction(ISD::SUBE, MVT::i32, Custom); + if (!Subtarget->isThumb1Only()) { + // FIXME: We should do this for Thumb1 as well. + setOperationAction(ISD::ADDC, MVT::i32, Custom); + setOperationAction(ISD::ADDE, MVT::i32, Custom); + setOperationAction(ISD::SUBC, MVT::i32, Custom); + setOperationAction(ISD::SUBE, MVT::i32, Custom); + } if (!Subtarget->isThumb1Only() && Subtarget->hasV6T2Ops()) setOperationAction(ISD::BITREVERSE, MVT::i32, Legal); @@ -9093,45 +9096,19 @@ void ARMTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI, // Rename pseudo opcodes. unsigned NewOpc = convertAddSubFlagsOpcode(MI.getOpcode()); - unsigned ccOutIdx; if (NewOpc) { const ARMBaseInstrInfo *TII = Subtarget->getInstrInfo(); MCID = &TII->get(NewOpc); - assert(MCID->getNumOperands() == - MI.getDesc().getNumOperands() + 5 - MI.getDesc().getSize() - && "converted opcode should be the same except for cc_out" - " (and, on Thumb1, pred)"); + assert(MCID->getNumOperands() == MI.getDesc().getNumOperands() + 1 && + "converted opcode should be the same except for cc_out"); MI.setDesc(*MCID); // Add the optional cc_out operand MI.addOperand(MachineOperand::CreateReg(0, /*isDef=*/true)); - - // On Thumb1, move all input operands to the end, then add the predicate - if (Subtarget->isThumb1Only()) { - for (unsigned c = MCID->getNumOperands() - 4; c--;) { - MI.addOperand(MI.getOperand(1)); - MI.RemoveOperand(1); - } - - // Restore the ties - for (unsigned i = MI.getNumOperands(); i--;) { - const MachineOperand& op = MI.getOperand(i); - if (op.isReg() && op.isUse()) { - int DefIdx = MCID->getOperandConstraint(i, MCOI::TIED_TO); - if (DefIdx != -1) - MI.tieOperands(DefIdx, i); - } - } - - MI.addOperand(MachineOperand::CreateImm(ARMCC::AL)); - MI.addOperand(MachineOperand::CreateReg(0, /*isDef=*/false)); - ccOutIdx = 1; - } else - ccOutIdx = MCID->getNumOperands() - 1; - } else - ccOutIdx = MCID->getNumOperands() - 1; + } + unsigned ccOutIdx = MCID->getNumOperands() - 1; // Any ARM instruction that sets the 's' bit should specify an optional // "cc_out" operand in the last operand position. @@ -9162,9 +9139,7 @@ void ARMTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI, if (deadCPSR) { assert(!MI.getOperand(ccOutIdx).getReg() && "expect uninitialized optional cc_out operand"); - // Thumb1 instructions must have the S bit even if the CPSR is dead. - if (!Subtarget->isThumb1Only()) - return; + return; } // 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, return SDValue(); } -static SDValue PerformAddcSubcCombine(SDNode *N, SelectionDAG &DAG, - const ARMSubtarget *Subtarget) { - if (Subtarget->isThumb1Only()) { - SDValue RHS = N->getOperand(1); - if (ConstantSDNode *C = dyn_cast(RHS)) { - int32_t imm = C->getSExtValue(); - if (imm < 0 && imm > -2147483648) { - SDLoc DL(N); - RHS = DAG.getConstant(-imm, DL, MVT::i32); - unsigned Opcode = (N->getOpcode() == ARMISD::ADDC) ? ARMISD::SUBC - : ARMISD::ADDC; - return DAG.getNode(Opcode, DL, N->getVTList(), N->getOperand(0), RHS); - } - } - } - return SDValue(); -} - -static SDValue PerformAddeSubeCombine(SDNode *N, SelectionDAG &DAG, - const ARMSubtarget *Subtarget) { - if (Subtarget->isThumb1Only()) { - SDValue RHS = N->getOperand(1); - if (ConstantSDNode *C = dyn_cast(RHS)) { - int64_t imm = C->getSExtValue(); - if (imm < 0) { - SDLoc DL(N); - - // The with-carry-in form matches bitwise not instead of the negation. - // Effectively, the inverse interpretation of the carry flag already - // accounts for part of the negation. - RHS = DAG.getConstant(~imm, DL, MVT::i32); - - unsigned Opcode = (N->getOpcode() == ARMISD::ADDE) ? ARMISD::SUBE - : ARMISD::ADDE; - return DAG.getNode(Opcode, DL, N->getVTList(), - N->getOperand(0), RHS, N->getOperand(2)); - } - } - } - return SDValue(); -} - /// PerformADDECombine - Target-specific dag combine transform from /// ARMISD::ADDC, ARMISD::ADDE, and ISD::MUL_LOHI to MLAL or /// ARMISD::ADDC, ARMISD::ADDE and ARMISD::UMLAL to ARMISD::UMAAL @@ -9834,7 +9767,7 @@ static SDValue PerformADDECombine(SDNode *N, const ARMSubtarget *Subtarget) { // Only ARM and Thumb2 support UMLAL/SMLAL. if (Subtarget->isThumb1Only()) - return PerformAddeSubeCombine(N, DCI.DAG, Subtarget); + return SDValue(); // Only perform the checks after legalize when the pattern is available. if (DCI.isBeforeLegalize()) return SDValue(); @@ -11934,9 +11867,6 @@ SDValue ARMTargetLowering::PerformDAGCombine(SDNode *N, case ISD::OR: return PerformORCombine(N, DCI, Subtarget); 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.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); case ARMISD::VMOVDRR: return PerformVMOVDRRCombine(N, DCI.DAG); diff --git a/llvm/lib/Target/ARM/ARMInstrThumb.td b/llvm/lib/Target/ARM/ARMInstrThumb.td index c372bcd..e6934cb 100644 --- a/llvm/lib/Target/ARM/ARMInstrThumb.td +++ b/llvm/lib/Target/ARM/ARMInstrThumb.td @@ -922,7 +922,7 @@ let isAdd = 1 in { def tADC : // A8.6.2 T1sItDPEncode<0b0101, (outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm), IIC_iALUr, "adc", "\t$Rdn, $Rm", - []>, Sched<[WriteALU]>; + [(set tGPR:$Rdn, (adde tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>; // Add immediate def tADDi3 : // A8.6.4 T1 @@ -950,43 +950,6 @@ let isAdd = 1 in { "add", "\t$Rd, $Rn, $Rm", [(set tGPR:$Rd, (add tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>; - /// Similar to the above except these set the 's' bit so the - /// instruction modifies the CPSR register. - /// - /// These opcodes will be converted to the real non-S opcodes by - /// AdjustInstrPostInstrSelection after giving then an optional CPSR operand. - let hasPostISelHook = 1, Defs = [CPSR] in { - let isCommutable = 1 in - def tADCS : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm), - 2, IIC_iALUr, - [(set tGPR:$Rdn, CPSR, (ARMadde tGPR:$Rn, tGPR:$Rm, - CPSR))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - - def tADDSi3 : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rm, imm0_7:$imm3), - 2, IIC_iALUi, - [(set tGPR:$Rd, CPSR, (ARMaddc tGPR:$Rm, - imm0_7:$imm3))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - - def tADDSi8 : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, imm0_255:$imm8), - 2, IIC_iALUi, - [(set tGPR:$Rdn, CPSR, (ARMaddc tGPR:$Rn, - imm8_255:$imm8))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - - let isCommutable = 1 in - def tADDSrr : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rn, tGPR:$Rm), - 2, IIC_iALUr, - [(set tGPR:$Rd, CPSR, (ARMaddc tGPR:$Rn, - tGPR:$Rm))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - } - let hasSideEffects = 0 in def tADDhirr : T1pIt<(outs GPR:$Rdn), (ins GPR:$Rn, GPR:$Rm), IIC_iALUr, "add", "\t$Rdn, $Rm", []>, @@ -1252,7 +1215,7 @@ def tSBC : // A8.6.151 T1sItDPEncode<0b0110, (outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm), IIC_iALUr, "sbc", "\t$Rdn, $Rm", - []>, + [(set tGPR:$Rdn, (sube tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>; // Subtract immediate @@ -1289,41 +1252,6 @@ def tSUBrr : // A8.6.212 [(set tGPR:$Rd, (sub tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>; -/// Similar to the above except these set the 's' bit so the -/// instruction modifies the CPSR register. -/// -/// These opcodes will be converted to the real non-S opcodes by -/// AdjustInstrPostInstrSelection after giving then an optional CPSR operand. -let hasPostISelHook = 1, Defs = [CPSR] in { - def tSBCS : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm), - 2, IIC_iALUr, - [(set tGPR:$Rdn, CPSR, (ARMsube tGPR:$Rn, tGPR:$Rm, - CPSR))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - - def tSUBSi3 : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rm, imm0_7:$imm3), - 2, IIC_iALUi, - [(set tGPR:$Rd, CPSR, (ARMsubc tGPR:$Rm, - imm0_7:$imm3))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - - def tSUBSi8 : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, imm0_255:$imm8), - 2, IIC_iALUi, - [(set tGPR:$Rdn, CPSR, (ARMsubc tGPR:$Rn, - imm8_255:$imm8))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; - - def tSUBSrr : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rn, tGPR:$Rm), - 2, IIC_iALUr, - [(set tGPR:$Rd, CPSR, (ARMsubc tGPR:$Rn, - tGPR:$Rm))]>, - Requires<[IsThumb1Only]>, - Sched<[WriteALU]>; -} - // Sign-extend byte def tSXTB : // A8.6.222 T1pIMiscEncode<{0,0,1,0,0,1,?}, (outs tGPR:$Rd), (ins tGPR:$Rm), @@ -1484,6 +1412,22 @@ def : T1Pat<(ARMcmpZ tGPR:$Rn, imm0_255:$imm8), def : T1Pat<(ARMcmpZ tGPR:$Rn, tGPR:$Rm), (tCMPr tGPR:$Rn, tGPR:$Rm)>; +// Add with carry +def : T1Pat<(addc tGPR:$lhs, imm0_7:$rhs), + (tADDi3 tGPR:$lhs, imm0_7:$rhs)>; +def : T1Pat<(addc tGPR:$lhs, imm8_255:$rhs), + (tADDi8 tGPR:$lhs, imm8_255:$rhs)>; +def : T1Pat<(addc tGPR:$lhs, tGPR:$rhs), + (tADDrr tGPR:$lhs, tGPR:$rhs)>; + +// Subtract with carry +def : T1Pat<(addc tGPR:$lhs, imm0_7_neg:$rhs), + (tSUBi3 tGPR:$lhs, imm0_7_neg:$rhs)>; +def : T1Pat<(addc tGPR:$lhs, imm8_255_neg:$rhs), + (tSUBi8 tGPR:$lhs, imm8_255_neg:$rhs)>; +def : T1Pat<(subc tGPR:$lhs, tGPR:$rhs), + (tSUBrr tGPR:$lhs, tGPR:$rhs)>; + // Bswap 16 with load/store def : T1Pat<(srl (bswap (extloadi16 t_addrmode_is2:$addr)), (i32 16)), (tREV16 (tLDRHi t_addrmode_is2:$addr))>; diff --git a/llvm/test/CodeGen/Thumb/long.ll b/llvm/test/CodeGen/Thumb/long.ll index c549bd4..2330adf 100644 --- a/llvm/test/CodeGen/Thumb/long.ll +++ b/llvm/test/CodeGen/Thumb/long.ll @@ -1,47 +1,33 @@ -; RUN: llc -mtriple=thumb-eabi %s -verify-machineinstrs -o - | FileCheck %s -; RUN: llc -mtriple=thumb-apple-darwin %s -verify-machineinstrs -o - | \ +; RUN: llc -mtriple=thumb-eabi %s -o - | FileCheck %s +; RUN: llc -mtriple=thumb-apple-darwin %s -o - | \ ; RUN: FileCheck %s -check-prefix CHECK -check-prefix CHECK-DARWIN define i64 @f1() { entry: ret i64 0 -; CHECK-LABEL: f1: -; CHECK: movs r0, #0 -; CHECK: movs r1, r0 } define i64 @f2() { entry: ret i64 1 -; CHECK-LABEL: f2: -; CHECK: movs r0, #1 -; CHECK: movs r1, #0 } define i64 @f3() { entry: ret i64 2147483647 -; CHECK-LABEL: f3: -; CHECK: ldr r0, -; CHECK: movs r1, #0 } define i64 @f4() { entry: ret i64 2147483648 -; CHECK-LABEL: f4: -; CHECK: movs r0, #1 -; CHECK: lsls r0, r0, #31 -; CHECK: movs r1, #0 } define i64 @f5() { entry: ret i64 9223372036854775807 ; CHECK-LABEL: f5: -; CHECK: movs r0, #0 -; CHECK: mvns r0, r0 -; CHECK: ldr r1, +; CHECK: mvn +; CHECK-NOT: mvn } define i64 @f6(i64 %x, i64 %y) { @@ -49,40 +35,14 @@ entry: %tmp1 = add i64 %y, 1 ; [#uses=1] ret i64 %tmp1 ; CHECK-LABEL: f6: -; CHECK: movs r1, #0 -; CHECK: adds r0, r2, #1 -; CHECK: adcs r1, r3 -} - -define i64 @f6a(i64 %x, i64 %y) { -entry: - %tmp1 = add i64 %y, 10 - ret i64 %tmp1 -; CHECK-LABEL: f6a: -; CHECK: movs r1, #0 -; CHECK: adds r2, #10 -; CHECK: adcs r1, r3 -; CHECK: movs r0, r2 -} - -define i64 @f6b(i64 %x, i64 %y) { -entry: - %tmp1 = add i64 %y, 1000 - ret i64 %tmp1 -; CHECK-LABEL: f6b: -; CHECK: movs r0, #125 -; CHECK: lsls r0, r0, #3 -; CHECK: movs r1, #0 -; CHECK: adds r0, r2, r0 -; CHECK: adcs r1, r3 +; CHECK: adc +; CHECK-NOT: adc } define void @f7() { entry: %tmp = call i64 @f8( ) ; [#uses=0] ret void -; CHECK-LABEL: f7: -; CHECK: bl } declare i64 @f8() @@ -92,56 +52,8 @@ entry: %tmp = sub i64 %a, %b ; [#uses=1] ret i64 %tmp ; CHECK-LABEL: f9: -; CHECK: subs r0, r0, r2 -; CHECK: sbcs r1, r3 -} - -define i64 @f9a(i64 %x, i64 %y) { ; ADDC with small negative imm => SUBS imm -entry: - %tmp1 = sub i64 %y, 10 - ret i64 %tmp1 -; CHECK-LABEL: f9a: -; CHECK: movs r0, #0 -; CHECK: subs r2, #10 -; CHECK: sbcs r3, r0 -; CHECK: movs r0, r2 -; CHECK: movs r1, r3 -} - -define i64 @f9b(i64 %x, i64 %y) { ; ADDC with big negative imm => SUBS reg -entry: - %tmp1 = sub i64 1000, %y - ret i64 %tmp1 -; CHECK-LABEL: f9b: -; CHECK: movs r0, #125 -; CHECK: lsls r0, r0, #3 -; CHECK: movs r1, #0 -; CHECK: subs r0, r0, r2 -; CHECK: sbcs r1, r3 -} - -define i64 @f9c(i64 %x, i32 %y) { ; SUBS with small positive imm => SUBS imm -entry: - %conv = sext i32 %y to i64 - %shl = shl i64 %conv, 32 - %or = or i64 %shl, 1 - %sub = sub nsw i64 %x, %or - ret i64 %sub -; CHECK-LABEL: f9c: -; CHECK: subs r0, r0, #1 -; CHECK: sbcs r1, r2 -} - -define i64 @f9d(i64 %x, i32 %y) { ; SUBS with small negative imm => ADDS imm -entry: - %conv = sext i32 %y to i64 - %shl = shl i64 %conv, 32 - %or = or i64 %shl, 4294967295 - %sub = sub nsw i64 %x, %or - ret i64 %sub -; CHECK-LABEL: f9d: -; CHECK: adds r0, r0, #1 -; CHECK: sbcs r1, r2 +; CHECK: sbc +; CHECK-NOT: sbc } define i64 @f(i32 %a, i32 %b) { @@ -151,7 +63,6 @@ entry: %tmp2 = mul i64 %tmp1, %tmp ; [#uses=1] ret i64 %tmp2 ; CHECK-LABEL: f: -; CHECK-V6: bl __aeabi_lmul ; CHECK-DARWIN: __muldi3 } @@ -162,7 +73,6 @@ entry: %tmp2 = mul i64 %tmp1, %tmp ; [#uses=1] ret i64 %tmp2 ; CHECK-LABEL: g: -; CHECK-V6: bl __aeabi_lmul ; CHECK-DARWIN: __muldi3 } @@ -171,38 +81,4 @@ entry: %a = alloca i64, align 8 ; [#uses=1] %retval = load i64, i64* %a ; [#uses=1] ret i64 %retval -; CHECK-LABEL: f10: -; CHECK: sub sp, #8 -; CHECK: ldr r0, [sp] -; CHECK: ldr r1, [sp, #4] -; CHECK: add sp, #8 -} - -define i64 @f11(i64 %x, i64 %y) { -entry: - %tmp1 = add i64 -1000, %y - %tmp2 = add i64 %tmp1, -1000 - ret i64 %tmp2 -; CHECK-LABEL: f11: -; CHECK: movs r0, #125 -; CHECK: lsls r0, r0, #3 -; CHECK: movs r1, #0 -; CHECK: subs r2, r2, r0 -; CHECK: sbcs r3, r1 -; CHECK: subs r0, r2, r0 -; CHECK: sbcs r3, r1 -; CHECK: movs r1, r3 -} - -; "sub 2147483648" has to be lowered into "add -2147483648" -define i64 @f12(i64 %x, i64 %y) { -entry: - %tmp1 = sub i64 %x, 2147483648 - ret i64 %tmp1 -; CHECK-LABEL: f12: -; CHECK: movs r2, #1 -; CHECK: lsls r2, r2, #31 -; CHECK: movs r3, #0 -; CHECK: adds r0, r0, r2 -; CHECK: sbcs r1, r3 } -- 2.7.4