The patch implements microMIPSr6 BREAK, EHB and EI instructions.
Details
Diff Detail
Event Timeline
lib/Target/Mips/MicroMips32r6InstrFormats.td | ||
---|---|---|
286 | Nit: It's called 'rs' in the docs. | |
lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
343 | Shouldn't EHB's R6MMR6Rel be StdMMR6Rel? | |
lib/Target/Mips/MipsInstrInfo.td | ||
410–413 | Adding a second uimm10 operand is going to get confusing. Please use the existing uimm10 and change that to restrict the accepted immediates like you have here. I'd recommend changing uimm10 in a separate commit. | |
test/MC/Disassembler/Mips/micromips.txt | ||
357–362 | I know this is from earlier patches, but please change the rest of this file (in a separate commit) to the 0x00 0x0a 0x57 0x7c # CHECK: ei $10 style. Could you also do this for the other micromips disassembler tests? |
lib/Target/Mips/MicroMips32r6InstrFormats.td | ||
---|---|---|
286 | I think there is some problem with docs. Tests pass with rt but drops with rs. There is also rt in format class EI_FM_MM in MicroMipsInstrFormats.td. Can you check, please. |
R6MMR6Rel is changed to StdMMR6Rel in definition of EHB instruction.
Existing uimm10 operand is used instead of new one.
Operand uimm10 is changed to restrict the accepted immediates in separate commit.
Text style in micromips disassembler test files (.txt) is changed to "one-line" style in separate commit.
LGTM with the comment added.
lib/Target/Mips/MicroMips32r6InstrFormats.td | ||
---|---|---|
286 | The failure when you change this isn't a documentation problem. Most likely, you changed this 'rt' to 'rs' but didn't update the 'rt' in EI_MMR6_DESC to match it. Operands are matched to encoding fields by name. That said, the documentation uses rt for MIPS and rs for microMIPS. Both documents seem correct, the difference in name stems from differences in how the encodings have been assigned to the encoding formats. I see that we're re-using the MIPS operation description and it would be a shame to duplicate it just to rename the operand. I therefore suggest adding a comment here. Maybe: bits<5> rt; // Actually rs but we're sharing code with the standard encodings which call it rt. | |
lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
343 | Done | |
lib/Target/Mips/MipsInstrInfo.td | ||
410–413 | Done |
Closed by commit rL240531: [mips][microMIPS] Implement BREAK, EHB and EI instructions
Nit: It's called 'rs' in the docs.