Defined a new register class for coprocessor 0's registers.
The instruction definitions for DMTC0, DMFC0, MFC0 and MTC0 have been changed to reflect this.
Paths
| Differential D10567
[mips] MFC0, MTC0 changes, COP0 register class definition. ClosedPublic Authored by sdardis on Jun 19 2015, 7:11 AM.
Details Summary Defined a new register class for coprocessor 0's registers. The instruction definitions for DMTC0, DMFC0, MFC0 and MTC0 have been changed to reflect this.
Diff Detail Event Timelinesdardis updated this object. Comment Actions The change looks correct to me but the patch lacks appropriate test cases. Could you add them?
sdardis edited edge metadata. Comment ActionsTests for mfc0, mtc0, and double word variants in the disassembler. Is there any else required? Thanks. Comment Actions Thanks. The tests need expanding on a bit (mostly copy/paste into each ISA's test file) and the commit message needs improving (particularly the subject line) but after that it will LGTM. I'm aware you don't have commit access yet, so could you update the revision and I'll commit from there. To elaborate on the tests that are needed: For the disassembler, the tests you've added need to be duplicated in each ISA's test (test/MC/Disassembler/Mips/$isa/valid-$isa.txt and test/MC/Disassembler/Mips/$isa/valid-$isa-el.txt for big/little endian respectively). I believe that mfc0/mtc0 also need to be included in the tests for the 64-bit ISA's since they are still valid instructions. The tests are laid out like this to make it easy to inspect the differences between the ISA's and manually check against documentation. These files should be sorted by opcode. For the assembler, we also need to test the encoding is correct. The existing tests for this are in test/MC/Mips/*/valid.s (one for each ISA). These files are sorted alphabetically to make it easy to verify against the documentation and inspect the ISA differences using diff. Lastly, we also need to add to test/MC/Mips/$isa1/invalid-$isa2.s which tests that $isa1 rejects things that were valid for $isa2. In this case, we just need to update the existing test/MC/Mips/*/invalid-mips64.s to check that dmfc0/dmtc0 are rejected with the correct error message. If they are rejected but with the wrong error message then there are also invalid-*-xfail.txt tests that can be used.
dsanders edited edge metadata. Comment ActionsLGTM. I'm aware you don't have commit access yet so I'll commit this shortly. I realize I said that disassembler tests are supposed to be sorted by opcode but it looks like they're sorted by mnemonic. You've made the correct change despite my mistake. Thanks. This revision is now accepted and ready to land.Jun 26 2015, 8:19 AM Comment Actions In the commit, I've fixed a couple issues that came up when running 'ninja check'.
I also marked DMFC0 as being part of MIPS3 like the other DM[FT]C0's.
Revision Contents
Diff 28556 lib/Target/Mips/AsmParser/MipsAsmParser.cpp
lib/Target/Mips/Disassembler/MipsDisassembler.cpp
lib/Target/Mips/MCTargetDesc/MipsOptionRecord.cpp
lib/Target/Mips/Mips64InstrInfo.td
lib/Target/Mips/MipsInstrInfo.td
lib/Target/Mips/MipsOptionRecord.h
lib/Target/Mips/MipsRegisterInfo.td
test/MC/Disassembler/Mips/mips32.txt
test/MC/Disassembler/Mips/mips32/valid-mips32-el.txt
test/MC/Disassembler/Mips/mips32/valid-mips32.txt
test/MC/Disassembler/Mips/mips32_le.txt
test/MC/Disassembler/Mips/mips32r2.txt
test/MC/Disassembler/Mips/mips32r2/valid-mips32r2-le.txt
test/MC/Disassembler/Mips/mips32r2/valid-mips32r2.txt
test/MC/Disassembler/Mips/mips32r2_le.txt
test/MC/Disassembler/Mips/mips32r3/valid-mips32r3-le.txt
test/MC/Disassembler/Mips/mips32r3/valid-mips32r3.txt
test/MC/Disassembler/Mips/mips32r5/valid-mips32r5-le.txt
test/MC/Disassembler/Mips/mips32r5/valid-mips32r5.txt
test/MC/Disassembler/Mips/mips32r6.txt
test/MC/Disassembler/Mips/mips32r6/valid-mips32r6-el.txt
test/MC/Disassembler/Mips/mips32r6/valid-mips32r6.txt
test/MC/Disassembler/Mips/mips64.txt
test/MC/Disassembler/Mips/mips64/valid-mips64-el.txt
test/MC/Disassembler/Mips/mips64/valid-mips64.txt
test/MC/Disassembler/Mips/mips64r2/valid-mips64r2-el.txt
test/MC/Disassembler/Mips/mips64r2/valid-mips64r2.txt
test/MC/Disassembler/Mips/mips64r3/valid-mips64r3-el.txt
test/MC/Disassembler/Mips/mips64r3/valid-mips64r3.txt
test/MC/Disassembler/Mips/mips64r5/valid-mips64r5-el.txt
test/MC/Disassembler/Mips/mips64r5/valid-mips64r5.txt
test/MC/Disassembler/Mips/mips64r6/valid-mips64r6-el.txt
test/MC/Disassembler/Mips/mips64r6/valid-mips64r6.txt
test/MC/Mips/mips-cop0-reginfo.s
test/MC/Mips/mips32/valid.s
test/MC/Mips/mips32r2/valid.s
test/MC/Mips/mips32r3/valid.s
test/MC/Mips/mips32r5/valid.s
test/MC/Mips/mips32r6/valid.s
test/MC/Mips/mips64/valid.s
test/MC/Mips/mips64r2/valid.s
test/MC/Mips/mips64r3/valid.s
test/MC/Mips/mips64r5/valid.s
test/MC/Mips/mips64r6/valid.s
|
Tiny nit: Could you add a small comment that the FPU is COP1? This will make the ordering of the if-statements obvious.