The patch implements microMIPSr6 LWM16, SB16, SH16, SW16, SWSP and SWM16 instructions.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Added support to register list for 64-bit registers.
Subsets of PredicateControl used instead of the Predicates list.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp | ||
---|---|---|
983–987 ↗ | (On Diff #34151) | Can this be written more clearly? It's rather difficult to follow. Also, why not use both front() and back() rather than mixing begin() and back()? |
2579 ↗ | (On Diff #34151) | ' || hasMips64r6()' is redundant since predicates are cumulative. |
3978–3979 ↗ | (On Diff #34151) | Do the registers mentioned here contain data or pointers? The reason I ask is that if it's a pointer then isGP64bit() isn't the right predicate (it should be ArePtrs64bit()) If the answer is 'either', then sticking with isGP64bit() is wrong but it's less wrong than ArePtrs64bit() and there's no right answer without knowing the data type. Likewise for the other registers below. |
4004–4011 ↗ | (On Diff #34151) | Can we write this more clearly? It's getting rather big and looks like it could be simpler. |
lib/Target/Mips/Disassembler/MipsDisassembler.cpp | ||
1873–1874 ↗ | (On Diff #34151) | Brace belongs on same line as the switch |
lib/Target/Mips/MicroMips32r6InstrFormats.td | ||
45 ↗ | (On Diff #34151) | Should begin with POOL16C |
lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
547 ↗ | (On Diff #34151) | This isn't for your patch but it would be good for 'mem_mm_4sp' to indicate the offset is unsigned. |
767 ↗ | (On Diff #34151) | Also not for your patch but it would be good for addrimm4lsl2 to indicate that it's unsigned too. |
lib/Target/Mips/MicroMipsInstrInfo.td | ||
530 ↗ | (On Diff #34151) | This belongs in the format class. Likewise below |
Used front() instead of begin() in isRegList16() method.
Removed hasMips64r6() from method expandLoadStoreMultiple() because it is not needed.
Registers may contain data so isGP64bit() is left in parseRegisterList() method.
Added POOL16C to format class name.
PredicateControl moved to format class LWM_FM_MM16.
LGTM. There was one unanswered question but after re-reading this patch I'm of the opinion that there's no correct answer and using isGP64bit() as the condition is the most correct answer possible.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp | ||
---|---|---|
4174–4176 ↗ | (On Diff #36508) |
I haven't seen an answer to this question but after re-reading I'm pretty sure the answer is 'either'. |