Added support for the following instructions: CACHEE, LBE, LBUE, LHE, LHUE, LWE, LLE, LWLE, LWRE, PREFE, SBE, SHE, SWE, SCE, SWLE, SWRE, TLBINV, TLBINVF This required adding some infrastructure for the EVA ASE.
Details
Diff Detail
Event Timeline
There's few comments here but it's all fairly minor things.
lib/Target/Mips/Disassembler/MipsDisassembler.cpp | ||
---|---|---|
245–247 | Nit: Indendation | |
1121–1123 | Nit: Indentation | |
1131–1132 | Nit: Indentation | |
lib/Target/Mips/MipsEVAInstrFormats.td | ||
17–20 | This belongs with the other INSN_* classes in MipsInstrInfo.td | |
24 | Spelling nit: I'm trying to keep the names of new format classes related to their names in the documentation (<major-opcode-name>_<arbitrary>_FM). Could you call this: SPECIAL3_EVA_LOAD_STORE_FM or similar? | |
38–53 | This is the same as LWE_FM above. Could you use that instead? | |
lib/Target/Mips/MipsEVAInstrInfo.td | ||
20 | Could you follow the conventions from Mips{DSP,MSA,32r6,64r6}InstrInfo.td? These conventions define concrete encodings in *_ENC classes, operation descriptions in *_DESC classes and apply predicates in the final def. This request is mostly spelling nits and syntax but there is also a change to the inheritance hierarchy. This change is that the format inherits from MipsInst (possibly via another class, see MipsR6Inst) instead of the operation description inheriting from it. | |
test/MC/Mips/eva/valid-xfail.s | ||
1 ↗ | (On Diff #29554) | The xfail status and filename of this are misleading. It's correct that EVA instructions fail to assemble when EVA is not enabled so it shouldn't be an xfail. I think you want a invalid-noeva.s with similar contents that checks the error message produced by each instruction. See invalid-mips1.s for an example. |
test/MC/Mips/eva/valid_R6-xfail.s | ||
1 ↗ | (On Diff #29554) | These aren't valid for 32r6/64r6. [ls]w[lr] and the EVA equivalents [ls]w[lr]e were removed in 32r6/64r6. I think you want an invalid-*.s to cover them |
LGTM with a minor change to the predicates.
lib/Target/Mips/MipsEVAInstrInfo.td | ||
---|---|---|
176–182 | HasEVA doesn't belong in AdditionalPredicates. Define a INSN_EVA_NOT_32R6_64R6 and use that instead of ISA_MIPS1_NOT_32R6_64R6. Also, drop the assignment to EncodingPredicates. There was a FIXME on the LWL/LWR/SWL/SWR indicating that it was probably a bug. | |
185 | Likewise, drop this EncodingPredicates too. |
Responded to reviewers comments. Also, changed 'mem' in operands to 'mem_simm9' in order to properly detect the upper bounds for these operands.
Nit: Indendation