The patch implements microMIPSr6 LDC1, SDC1, LDC2, SDC2, LWC1, SWC1, LWC2 and SWC2 instructions and adds CodeGen support.
Details
Diff Detail
Event Timeline
Could you add some codegen test cases for the new instructions?
By the way, was leaving LDC1/LDC2 out of this patch intentional?
lib/Target/Mips/Disassembler/MipsDisassembler.cpp | ||
---|---|---|
1599–1614 | This function is the same as DecodeFMem but with the Reg and Base fields swapped. Could you add a comment saying that this is intentional and correct? | |
lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
660 | addrDefault does not permit offsets but the instruction supports 11-bit. See addrimm12 for an example that supports 12-bit offsets. | |
674 | addrDefault -> addr to support the 16-bit offset the instruction allows | |
688 | As per the LWC2 case, we should use an 11-bit version of addrimm12 to support the 11-bit offset the instruction allows | |
lib/Target/Mips/MicroMipsInstrFPU.td | ||
145–152 | The DecodeNamespace and Predicates/AdditionalPredicates change makes sense but the DecoderMethod change looks wrong. It's applying microMIPSR6 specific decoder methods to microMIPSR2 instructions. |
Moved implementation of LDC1 and LDC2 to this patch.
Added addrimm11 and addrimm16 complex pattern definitions.
Added DAG patterns (LoadRegImmPat and StoreRegImmPat).
Added operand checkings for LDC*, SDC*, LWC* and SWC* instructions.
Updated isMemWithSimmOffset method for relocation support.
Added tests for the standard encodings and invalid tests.
Updated .ll files with CodeGen tests for microMIPSr6.
Added micromips-lwc1-swc1.ll with CodeGen tests for microMIPSr6.
LGTM with some nits (inlined).
You'll need to add -target-abi n64 for any tests that are targeting micromips64r6, as some of the defaults have changed.
lib/Target/Mips/Mips32r6InstrInfo.td | ||
---|---|---|
758 | Rather than moving this line up, can you keep the existing ordering and wrap that line in a NotInMicroMips predicate? | |
test/CodeGen/Mips/mno-ldc1-sdc1.ll | ||
10 ↗ | (On Diff #59832) | Since this patch was first posted, FileCheck gained a new option -check-prefixes=. See rL273669 on how to use it. |
test/MC/Disassembler/Mips/micromips32r3/valid-el.txt | ||
192 | You can eliminate the near duplicates of what you're adding unless the encoding is some how irregular. i.e. the register field is split into two parts. This goes for the other disassembler tests you're adding. | |
test/MC/Mips/mips32r6/invalid.s | ||
115 ↗ | (On Diff #59832) | This test file also requires [ls][wd]dc2/ out of range tests. |
test/MC/Mips/mips64r6/invalid.s | ||
107 ↗ | (On Diff #59832) | This test file also requires [ls][wd]dc2/ out of range tests. |
This function is the same as DecodeFMem but with the Reg and Base fields swapped. Could you add a comment saying that this is intentional and correct?