[PATCH] Implement restrictions on SP in register list on LDM, LDMIA, LDMFD for ARM v7m

[Tim Northover]

================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6000
@@ -5997,1 +5999,3 @@
                    "writeback register not allowed in register list");
+    if (Opcode == ARM::t2LDMIA_UPD) {
+      if (listContainsReg(Inst, 4, ARM::SP))
----------------
jyoti.allur wrote:
> jyoti.allur wrote:
> > t.p.northover wrote:
> > > This restriction seems to apply to all the Thumb2 instructions in this case, not just t2LDMIA_UPD (but watch out for the fallthrough from ARM above).
> > > 
> > > It also applies to the non-updating variants (you may have to add extra cases).
> > yes, this restriction does apply to all thumb2 instructions in this case, i was planning on adding multiple patches for each set of instructions Section wise as mentioned in the ARM ARM, if that's okay with you?.
> > 
> okay, I will include non-updating variants namely ARM::t2LDMIA, ARM::t2LDMDB,ARM::t2STMIA,ARM::t2STMDB
> 
> What about thumb versions ARM::tLDMIA, ARM::tLDMDB,ARM::tSTMIA,ARM::tSTMDB
> i should include them as well rite?
> 
That'd be good. I don't think it's necessary to go through separate patches & review for the lot. It's really just one big issue: "LLVM doesn't know sp is forbidden in thumb ldm/stm instructions"

================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6001
@@ +6000,3 @@
+    if (Opcode == ARM::t2LDMIA_UPD) {
+      if (listContainsReg(Inst, 4, ARM::SP))
+        return Error(Operands.back()->getStartLoc(),
----------------
jyoti.allur wrote:
> jyoti.allur wrote:
> > t.p.northover wrote:
> > > The list doesn't start at operand 4 here, does it? It looks like it should be 3 again on my tests.
> > in some calls to checkLowRegisterList ( though this is a different function from listContainsReg, logic to read registers from list is same for both) operand 4 is used whenever a writeback expression exists in the instruction, since this case covered *_UPD case, 4 was used, correct me if my understanding is wrong.
> i will change it to operand number 3 when checking for SP, although i am not sure why 4 is used in checkLowRegisterList in some cases.
It depends on the exact instruction. I think STMs tend to start at 4, because they duplicate the address register for codegen purposes (once to be read, once to be written). The way to check is

    $ echo 'stmia r3!, {r0, r1}' | bin/llvm-mc -triple thumbv7 -show-inst
            stm	r3!, {r0, r1}           @ <MCInst #2769 tSTMIA_UPD
                                        @  <MCOperand Reg:69>
                                        @  <MCOperand Reg:69>
                                        @  <MCOperand Imm:14>
                                        @  <MCOperand Reg:0>
                                        @  <MCOperand Reg:66>
                                        @  <MCOperand Reg:67>>

You see a duplicated 69 (the r3), a couple of predicate related operands, followed by the real register list starting at operand 4 (for tSTMIA_UPD). Hence line 6054.

http://reviews.llvm.org/D5827