This is an archive of the discontinued LLVM Phabricator instance.

[mips] Added support for various EVA ASE instructions.
ClosedPublic

Authored by s.egerton on Jul 13 2015, 6:00 AM.

Details

Summary
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.

Diff Detail

Event Timeline

s.egerton updated this revision to Diff 29554.Jul 13 2015, 6:00 AM
s.egerton retitled this revision from to [mips] Added support for various EVA ASE instructions..
s.egerton updated this object.
s.egerton added reviewers: dsanders, vkalintiris.
s.egerton added a subscriber: llvm-commits.
dsanders edited edge metadata.Jul 20 2015, 6:48 AM

There's few comments here but it's all fairly minor things.

lib/Target/Mips/Disassembler/MipsDisassembler.cpp
245–247

Nit: Indendation

1099–1101

Nit: Indentation

1109–1110

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

s.egerton updated this revision to Diff 30360.Jul 22 2015, 9:02 AM
s.egerton marked 6 inline comments as done.
s.egerton edited edge metadata.

Responded to reviewers comments.

dsanders accepted this revision.Sep 7 2015, 5:50 AM
dsanders edited edge metadata.

LGTM with a minor change to the predicates.

lib/Target/Mips/MipsEVAInstrInfo.td
175–181

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.

184

Likewise, drop this EncodingPredicates too.

This revision is now accepted and ready to land.Sep 7 2015, 5:50 AM
s.egerton updated this revision to Diff 34306.Sep 9 2015, 3:03 AM
s.egerton marked 2 inline comments as done.
s.egerton edited edge metadata.

Responded to reviewers comments. Also, changed 'mem' in operands to 'mem_simm9' in order to properly detect the upper bounds for these operands.

dsanders closed this revision.Sep 15 2015, 3:03 AM