For compatiblity with binutils, define these instructions to take
two registers with a 16bit unsigned immediate. Both of the registers
have to be same for dahi and dati.
Details
- Reviewers
dsanders vkalintiris zoran.jovanovic
Diff Detail
Event Timeline
lib/Target/Mips/MicroMips64r6InstrInfo.td | ||
---|---|---|
93–97 | I think we need to split this out into a separate issue. I'm not very keen on the idea of working around a bug in the way tablegen handles tied operands, especially given that the workaround will prevent us from adding CodeGen support. As you mention in the comment, the issue is that CVT_Tied is implemented such that it copies the MCOperand that this operand is tied to without checking if the current operand would render to the same thing. As a result, the matcher silently ignores bad input and emits a valid instruction that isn't what the user specified. It should error out instead. I'll see if I can find a way to fix tablegen. |
This will need reworking to account for D21994 having been committed. I'll review this once it's been rebased.
I have a couple minor comments, the main one is the dubious disassembler test.
lib/Target/Mips/Disassembler/MipsDisassembler.cpp | ||
---|---|---|
601–604 | GPR32 -> GPR64 | |
lib/Target/Mips/Mips32r6InstrInfo.td | ||
319 | It feels wrong to have a simm16 immediate in the instruction but only accept uimm16 inputs in the assembly but I agree that this is the intended behaviour. I think we should render it to the instruction as a simm16 so that the MCInst's are consistent though. | |
test/MC/Disassembler/Mips/micromips64r6/valid.txt | ||
38–40 | This doesn't look correct to me. I haven't double-checked the encodings but there's only one bit different between the two instructions yet two things are different in the assembly (dahi->dati, and $16 -> $17) Also, could you comment on why it isn't $3 any more? |
test/MC/Disassembler/Mips/micromips64r6/valid.txt | ||
---|---|---|
38–40 | The change was due to a bug in the decoder, it was reading the minor opcode as the register for microMIPS64r6. MIPSR6 has the fields swapped in comparison. |
Sorry for the delay, I thought I'd redirected this temporary email address to my new one but apparently not
GPR32 -> GPR64