Implement restrictions on SP in register list to LDM, STM variants in thumb mode as specified by ARM ARM
Referred ARM for v7m
Details
Details
- Reviewers
t.p.northover echristo asl
Diff Detail
Diff Detail
Event Timeline
Comment Actions
Hi Jyoti,
Thanks very much for working on this (especially as it's one on my list to get around to). I think there are a few more cases to cover:
| llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | ||
|---|---|---|
| 6000 | 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). | |
| 6001 | The list doesn't start at operand 4 here, does it? It looks like it should be 3 again on my tests. | |
| llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | ||
|---|---|---|
| 6000 | 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?. | |
| 6001 | 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. | |
Comment Actions
Hi Tim,
I have replied inline again. LDMEA and STMEA stack operations are not included, should we consider them?
| llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | ||
|---|---|---|
| 6000 | 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 | |
| 6001 | 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. | |
| llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | ||
|---|---|---|
| 6000 | 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" | |
| 6001 | 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. | |
Comment Actions
Oops...
| llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | ||
|---|---|---|
| 6001 | I'm really sorry, you seem to have been right first time. Quite what test I did yesterday, I don't know. But today's one shows that t2LDMIA_UPD *does* start the list at 4. | |
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).