From 7e32ae6cf71f3f6d71be93285eb952c51c5e5f53 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Thu, 4 Dec 2014 14:10:20 +0000 Subject: [PATCH] Revert "[Thumb/Thumb2] Added restrictions on PC, LR, SP in the register list for PUSH/POP/LDM/STM. " This reverts commit r223356. It was failing check-all (MC/ARM/thumb.s in particular). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@223363 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 234 ++++++++-------------- test/MC/ARM/thumb-diagnostics.s | 32 +-- test/MC/ARM/thumb2-diagnostics.s | 6 - test/MC/ARM/v8_IT_manual.s | 7 +- 4 files changed, 96 insertions(+), 183 deletions(-) diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp index 97e1b23932e..1e661d82dad 100644 --- a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp +++ b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp @@ -174,11 +174,6 @@ class ARMAsmParser : public MCTargetAsmParser { ITState.CurPosition = ~0U; // Done with the IT block after this. } - bool lastInITBlock() { - unsigned TZ = countTrailingZeros(ITState.Mask); - return (ITState.CurPosition == 4 - TZ); - } - void Note(SMLoc L, const Twine &Msg, ArrayRef Ranges = None) { return getParser().Note(L, Msg, Ranges); } @@ -327,8 +322,6 @@ class ARMAsmParser : public MCTargetAsmParser { bool processInstruction(MCInst &Inst, const OperandVector &Ops, MCStreamer &Out); bool shouldOmitCCOutOperand(StringRef Mnemonic, OperandVector &Operands); bool shouldOmitPredicateOperand(StringRef Mnemonic, OperandVector &Operands); - bool validateRegListOperands(MCInst &Inst, const OperandVector &Operands, - unsigned OpNo); public: enum ARMMatchResultTy { @@ -5984,138 +5977,13 @@ static bool checkLowRegisterList(MCInst Inst, unsigned OpNo, unsigned Reg, return false; } -// Check if there are any special registers in the register list of the inst, +// Check if the specified regisgter is in the register list of the inst, // starting at the indicated operand number. -static void findSpecialRegsInList(MCInst &Inst, unsigned OpNo, bool &SP, - bool &PC, bool &LR, bool &BaseReg) { - SP = PC = LR = BaseReg = false; - unsigned Rn = Inst.getOperand(0).getReg(); +static bool listContainsReg(MCInst &Inst, unsigned OpNo, unsigned Reg) { for (unsigned i = OpNo; i < Inst.getNumOperands(); ++i) { unsigned OpReg = Inst.getOperand(i).getReg(); - if (OpReg == ARM::SP) - SP = true; - else if (OpReg == ARM::PC) - PC = true; - else if (OpReg == ARM::LR) - LR = true; - if (OpReg == Rn) - BaseReg = true; - } - - return; -} - -bool ARMAsmParser::validateRegListOperands(MCInst &Inst, - const OperandVector &Operands, - unsigned OpNo) { - bool SP, PC, LR, BaseReg, LowReg, listContainsBase = false; - const unsigned Opcode = Inst.getOpcode(); - unsigned Rn = Inst.getOperand(0).getReg(); - findSpecialRegsInList(Inst, OpNo, SP, PC, LR, BaseReg); - LowReg = checkLowRegisterList(Inst, OpNo, Rn, 0, listContainsBase); - - switch (Opcode) { - case ARM::t2LDMIA_UPD: - case ARM::t2LDMDB_UPD: { - if (BaseReg) - return Error(Operands.back()->getStartLoc(), - "writeback register not allowed in register list"); - } - case ARM::tLDMIA: { - // If we're parsing Thumb2, the .w variant is available and handles - // most cases that are normally illegal for a Thumb1 LDM instruction. - // We'll make the transformation in processInstruction() if necessary. - // - // Thumb LDM instructions are writeback iff the base register is not - // in the register list. - bool HasWritebackToken = - (static_cast(*Operands[3]).isToken() && - static_cast(*Operands[3]).getToken() == "!"); - - if (LowReg && !isThumbTwo()) - return Error(Operands[3 + HasWritebackToken]->getStartLoc(), - "registers must be in range r0-r7"); - // If we should have writeback, then there should be a '!' token. - if (!BaseReg && !HasWritebackToken && !isThumbTwo()) - return Error(Operands[2]->getStartLoc(), - "writeback operator '!' expected"); - - // If we should not have writeback, there must not be a '!'. This is - // true even for the 32-bit wide encodings. - if (BaseReg && HasWritebackToken) - return Error(Operands[3]->getStartLoc(), - "writeback operator '!' not allowed when base register " - "in register list"); - } - case ARM::tPOP: { - if (LowReg && !PC && !isThumbTwo()) - return Error(Operands[2]->getStartLoc(), - "registers must be in range r0-r7 or pc"); - } - case ARM::t2LDMIA: - case ARM::t2LDMDB: { - if (SP) - return Error(Operands[OpNo]->getStartLoc(), - "SP not allowed in register list"); - if (PC && LR) - return Error( - Operands[OpNo]->getStartLoc(), - "LR not allowed in the list, when PC is in the register list"); - if (PC && inITBlock() && !lastInITBlock()) - return Error(Operands[OpNo]->getStartLoc(), - "Instruction should be outside an IT block or last in IT " - "block, when PC is in the register list"); - break; - } - case ARM::t2STMIA_UPD: - case ARM::t2STMDB_UPD: { - if (BaseReg) - return Error(Operands.back()->getStartLoc(), - "writeback register not allowed in register list"); - } - case ARM::tSTMIA_UPD: { - if (LowReg && !isThumbTwo()) - return Error(Operands[4]->getStartLoc(), - "registers must be in range r0-r7"); - - // This would be converted to a 32-bit stm, but that's not valid if the - // writeback register is in the list. - if (LowReg && BaseReg) - return Error(Operands[4]->getStartLoc(), - "writeback operator '!' not allowed when base register " - "in register list"); - } - case ARM::tPUSH: { - if (LowReg && !LR && !isThumbTwo()) - return Error(Operands[2]->getStartLoc(), - "registers must be in range r0-r7 or lr"); - } - case ARM::t2STMIA: - case ARM::t2STMDB: { - if (SP || PC) - return Error(Operands[OpNo]->getStartLoc(), - "SP, PC not allowed in register list"); - break; - } - case ARM::LDMIA_UPD: - case ARM::LDMDB_UPD: - case ARM::LDMIB_UPD: - case ARM::LDMDA_UPD: { - if (BaseReg) - return Error(Operands.back()->getStartLoc(), - "writeback register not allowed in register list"); - break; - } - case ARM::sysLDMIA_UPD: - case ARM::sysLDMDA_UPD: - case ARM::sysLDMDB_UPD: - case ARM::sysLDMIB_UPD: { - if (!PC) - return Error(Operands[4]->getStartLoc(), - "writeback register only allowed on system LDM " - "if PC in register-list"); - break; - } + if (OpReg == Reg) + return true; } return false; } @@ -6287,6 +6155,37 @@ bool ARMAsmParser::validateInstruction(MCInst &Inst, "bitfield width must be in range [1,32-lsb]"); return false; } + // Notionally handles ARM::tLDMIA_UPD too. + case ARM::tLDMIA: { + // If we're parsing Thumb2, the .w variant is available and handles + // most cases that are normally illegal for a Thumb1 LDM instruction. + // We'll make the transformation in processInstruction() if necessary. + // + // Thumb LDM instructions are writeback iff the base register is not + // in the register list. + unsigned Rn = Inst.getOperand(0).getReg(); + bool HasWritebackToken = + (static_cast(*Operands[3]).isToken() && + static_cast(*Operands[3]).getToken() == "!"); + bool ListContainsBase; + if (checkLowRegisterList(Inst, 3, Rn, 0, ListContainsBase) && !isThumbTwo()) + return Error(Operands[3 + HasWritebackToken]->getStartLoc(), + "registers must be in range r0-r7"); + // If we should have writeback, then there should be a '!' token. + if (!ListContainsBase && !HasWritebackToken && !isThumbTwo()) + return Error(Operands[2]->getStartLoc(), + "writeback operator '!' expected"); + // If we should not have writeback, there must not be a '!'. This is + // true even for the 32-bit wide encodings. + if (ListContainsBase && HasWritebackToken) + return Error(Operands[3]->getStartLoc(), + "writeback operator '!' not allowed when base register " + "in register list"); + if (listContainsReg(Inst, 3 + HasWritebackToken, ARM::SP)) + return Error(Operands[3 + HasWritebackToken]->getStartLoc(), + "SP not allowed in register list"); + break; + } case ARM::LDMIA_UPD: case ARM::LDMDB_UPD: case ARM::LDMIB_UPD: @@ -6295,22 +6194,41 @@ bool ARMAsmParser::validateInstruction(MCInst &Inst, // UNPREDICTABLE on v7 upwards. Goodness knows what they did before. if (!hasV7Ops()) break; - case ARM::tLDMIA: // Notionally handles ARM::tLDMIA_UPD too. + if (listContainsReg(Inst, 3, Inst.getOperand(0).getReg())) + return Error(Operands.back()->getStartLoc(), + "writeback register not allowed in register list"); + break; case ARM::t2LDMIA: case ARM::t2LDMDB: case ARM::t2STMIA: - case ARM::t2STMDB: + case ARM::t2STMDB: { + if (listContainsReg(Inst, 3, ARM::SP)) + return Error(Operands.back()->getStartLoc(), + "SP not allowed in register list"); + break; + } case ARM::t2LDMIA_UPD: case ARM::t2LDMDB_UPD: case ARM::t2STMIA_UPD: - case ARM::t2STMDB_UPD: + case ARM::t2STMDB_UPD: { + if (listContainsReg(Inst, 3, Inst.getOperand(0).getReg())) + return Error(Operands.back()->getStartLoc(), + "writeback register not allowed in register list"); + + if (listContainsReg(Inst, 4, ARM::SP)) + return Error(Operands.back()->getStartLoc(), + "SP not allowed in register list"); + break; + } case ARM::sysLDMIA_UPD: case ARM::sysLDMDA_UPD: case ARM::sysLDMDB_UPD: - case ARM::sysLDMIB_UPD: { - validateRegListOperands(Inst, Operands, 3); + case ARM::sysLDMIB_UPD: + if (!listContainsReg(Inst, 3, ARM::PC)) + return Error(Operands[4]->getStartLoc(), + "writeback register only allowed on system LDM " + "if PC in register-list"); break; - } case ARM::sysSTMIA_UPD: case ARM::sysSTMDA_UPD: case ARM::sysSTMDB_UPD: @@ -6338,13 +6256,39 @@ bool ARMAsmParser::validateInstruction(MCInst &Inst, // Like for ldm/stm, push and pop have hi-reg handling version in Thumb2, // so only issue a diagnostic for thumb1. The instructions will be // switched to the t2 encodings in processInstruction() if necessary. - case ARM::tPOP: + case ARM::tPOP: { + bool ListContainsBase; + if (checkLowRegisterList(Inst, 2, 0, ARM::PC, ListContainsBase) && + !isThumbTwo()) + return Error(Operands[2]->getStartLoc(), + "registers must be in range r0-r7 or pc"); + break; + } case ARM::tPUSH: { - validateRegListOperands(Inst, Operands, 2); + bool ListContainsBase; + if (checkLowRegisterList(Inst, 2, 0, ARM::LR, ListContainsBase) && + !isThumbTwo()) + return Error(Operands[2]->getStartLoc(), + "registers must be in range r0-r7 or lr"); break; } case ARM::tSTMIA_UPD: { - validateRegListOperands(Inst, Operands, 4); + bool ListContainsBase, InvalidLowList; + InvalidLowList = checkLowRegisterList(Inst, 4, Inst.getOperand(0).getReg(), + 0, ListContainsBase); + if (InvalidLowList && !isThumbTwo()) + return Error(Operands[4]->getStartLoc(), + "registers must be in range r0-r7"); + + // This would be converted to a 32-bit stm, but that's not valid if the + // writeback register is in the list. + if (InvalidLowList && ListContainsBase) + return Error(Operands[4]->getStartLoc(), + "writeback operator '!' not allowed when base register " + "in register list"); + if (listContainsReg(Inst, 4, ARM::SP) && !inITBlock()) + return Error(Operands.back()->getStartLoc(), + "SP not allowed in register list"); break; } case ARM::tADDrSP: { diff --git a/test/MC/ARM/thumb-diagnostics.s b/test/MC/ARM/thumb-diagnostics.s index a1ad3c085b6..d2261293378 100644 --- a/test/MC/ARM/thumb-diagnostics.s +++ b/test/MC/ARM/thumb-diagnostics.s @@ -68,7 +68,6 @@ error: invalid operand for instruction ldmfd r2!, {r1, r3-r6, sp} ldmdb r1, {r2, r3, sp} ldmdb r1!, {r2, r3, sp} - ldm r2, {r5, lr, pc} @ CHECK-ERRORS: error: registers must be in range r0-r7 @ CHECK-ERRORS: ldm r2!, {r5, r8} @ CHECK-ERRORS: ^ @@ -105,31 +104,16 @@ error: invalid operand for instruction @ CHECK-ERRORS-V7M: error: SP not allowed in register list @ CHECK-ERRORS-V7M: ldmdb r1!, {r2, r3, sp} @ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: LR not allowed in the list, when PC is in the register list -@ CHECK-ERRORS-V7M: ldm r2, {r5, lr, pc} -@ CHECK-ERRORS-V7M: ^ @ Invalid writeback and register lists for PUSH/POP pop {r1, r2, r10} - pop {r1, r2, lr, pc} push {r8, r9} - push {r8, r9, sp} - push {r8, r9, pc} @ CHECK-ERRORS: error: registers must be in range r0-r7 or pc @ CHECK-ERRORS: pop {r1, r2, r10} @ CHECK-ERRORS: ^ -@ CHECK-ERRORS-V7M: error: LR not allowed in the list, when PC is in the register list -@ CHECK-ERRORS-V7M: pop {r1, r2, lr, pc} -@ CHECK-ERRORS-V7M: ^ @ CHECK-ERRORS: error: registers must be in range r0-r7 or lr @ CHECK-ERRORS: push {r8, r9} @ CHECK-ERRORS: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list -@ CHECK-ERRORS-V7M: push {r8, r9, sp} -@ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list -@ CHECK-ERRORS-V7M: push {r8, r9, pc} -@ CHECK-ERRORS-V7M: ^ @ Invalid writeback and register lists for STM @@ -141,8 +125,6 @@ error: invalid operand for instruction stmia r4!, {r0-r3, sp} stmdb r1, {r2, r3, sp} stmdb r1!, {r2, r3, sp} - stmia r4, {r2, sp, pc} - stmdb r1!, {r2, r3, pc} @ CHECK-ERRORS: error: instruction requires: thumb2 @ CHECK-ERRORS: stm r1, {r2, r6} @ CHECK-ERRORS: ^ @@ -155,24 +137,18 @@ error: invalid operand for instruction @ CHECK-ERRORS-V8: error: writeback register not allowed in register list @ CHECK-ERRORS-V8: stmdb r2!, {r0, r2} @ CHECK-ERRORS-V8: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list +@ CHECK-ERRORS-V7M: error: SP not allowed in register list @ CHECK-ERRORS-V7M: stm r1!, {r2, sp} @ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list +@ CHECK-ERRORS-V7M: error: SP not allowed in register list @ CHECK-ERRORS-V7M: stmia r4!, {r0-r3, sp} @ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list +@ CHECK-ERRORS-V7M: error: SP not allowed in register list @ CHECK-ERRORS-V7M: stmdb r1, {r2, r3, sp} @ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list +@ CHECK-ERRORS-V7M: error: SP not allowed in register list @ CHECK-ERRORS-V7M: stmdb r1!, {r2, r3, sp} @ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list -@ CHECK-ERRORS-V7M: stmia r4, {r2, sp, pc} -@ CHECK-ERRORS-V7M: ^ -@ CHECK-ERRORS-V7M: error: SP, PC not allowed in register list -@ CHECK-ERRORS-V7M: stmdb r1!, {r2, r3, pc} -@ CHECK-ERRORS-V7M: ^ @ Out of range immediates for LSL instruction. lsls r4, r5, #-1 diff --git a/test/MC/ARM/thumb2-diagnostics.s b/test/MC/ARM/thumb2-diagnostics.s index 5376c324069..b2b14bc13d6 100644 --- a/test/MC/ARM/thumb2-diagnostics.s +++ b/test/MC/ARM/thumb2-diagnostics.s @@ -9,9 +9,6 @@ iteeee gt ittfe le nopeq - ite eq - ldmiaeq sp!, {r4, pc} - movne r0, #0 @ CHECK-ERRORS: error: incorrect condition in IT block; got 'le', but expected 'eq' @ CHECK-ERRORS: addle r0, r1, r2 @@ -31,9 +28,6 @@ @ CHECK-ERRORS: error: predicated instructions must be in IT block @ CHECK-ERRORS: nopeq @ CHECK-ERRORS: ^ -@ CHECK-ERRORS: error: Instruction should be outside an IT block or last in IT block, when PC is in the register list -@ CHECK-ERRORS: ldmiaeq sp!, {r4, pc} -@ CHECK-ERRORS: ^ @ Out of range immediates for MRC/MRC2/MRRC/MRRC2 mrc p14, #8, r1, c1, c2, #4 diff --git a/test/MC/ARM/v8_IT_manual.s b/test/MC/ARM/v8_IT_manual.s index f6d3941cae2..4b63aa82dd6 100644 --- a/test/MC/ARM/v8_IT_manual.s +++ b/test/MC/ARM/v8_IT_manual.s @@ -554,11 +554,11 @@ pushge {r1, r3, r7} @ PUSH, encoding T2 (32-bit) @ CHECK: [[@LINE+2]]:1: warning: deprecated instruction in IT block it ge -pushge {r1, r3, r7} +pushge {r1, r13, r7} @ PUSH, encoding T3 (32-bit) @ CHECK: [[@LINE+2]]:1: warning: deprecated instruction in IT block it ge -pushge {r3} +pushge {r13} @ REV, encoding T1 @ CHECK: [[@LINE+2]]:1: warning: deprecated instruction in IT block @@ -614,10 +614,9 @@ stmge r1!, {r2, r3} @ CHECK: [[@LINE+2]]:1: warning: deprecated instruction in IT block it ge stmge r1, {r2, r3} -@ STM, encoding T3 (32-bit) @ CHECK: [[@LINE+2]]:1: warning: deprecated instruction in IT block it ge -stmge r1!, {r2} +stmge r1!, {r2, r13} @ LDM, encoding T1 @ CHECK: [[@LINE+2]]:1: warning: deprecated instruction in IT block -- 2.34.1