[RISCV] Add MC support of RISCV zcmt Extension

[VincentWu]

Summary

This patch add the instructions of zcmt extension.
spac is here
Which includes two instructions (cm.jt&cm.jalt) and a CSR Reg JVT

co-author: @Scott Egerton

[VincentWu]

fix test case

[craig.topper]

llvm/test/MC/RISCV/rv32zcmt-Invalid.s

5 ↗

(On Diff #460266)

Why no invalid test for cm.jalt?

llvm/test/MC/RISCV/rv32zcmt-valid.s

9

What about CHECK-NO-EXT like other tests have?

llvm/test/MC/RISCV/rv64zcmt-Invalid.s

1 ↗

(On Diff #460266)

Why is Invalid capitalized in this test name. It isn't capitalized in other MC tests is it?

1 ↗

(On Diff #460266)

Merge this with the rv32 test.

llvm/test/MC/RISCV/rv64zcmt-valid.s

1 ↗

(On Diff #460266)

Merge this with rv32zcmt-valid.s

[VincentWu]

update & address comments

[reames]

Please add to the review description a link to the appropriate specification. Please update docs/RISCVUsage.rst to add Zcmt, and link to the specification. It's impossible to review e.g. encoding without knowing what you're implementing.

llvm/lib/Target/RISCV/RISCV.td

367

Wait, Zcmt can't be enabled if C is? That seems odd...

llvm/lib/Target/RISCV/RISCVInstrInfoZc.td

1 ↗

(On Diff #463097)

We have Zca in RISCVInstrInfoC.td. Not sure it makes sense to have Zcmt in one place, and Zca in another.

Also, did Zca change between 0.7 and 0.7.5? If so, any plans to update that extension?

10 ↗

(On Diff #463097)

It's not just one extension. It is several sub-extensions.

Please reword as "from the Zc* family of code size reduction extensions, version ...

[craig.topper]

llvm/lib/Target/RISCV/RISCV.td

367

I think Zcmt reuses encodings from the FP part of C. That's why C was split into Zca, Zcf, and Zcd.

[craig.topper]

llvm/lib/Target/RISCV/RISCV.td

367

This restriction needs to be enforced in RISCVISAInfo.cpp so that it generates a proper error message.

llvm/lib/Target/RISCV/RISCVInstrInfoZc.td

1 ↗

(On Diff #463097)

I think all Zc* should be in RISCVInstrInfoC.td. Zca/Zcf/Zcd already have to be in that file. Might as well put the rest there.

We should mention the Zc* extensions in the header of that file.

llvm/lib/Target/RISCV/RISCVSystemOperands.td

386

This needs an assembler test

[jrtc27]

llvm/lib/Target/RISCV/RISCV.td

366

This is an odd implication, Zcmt works just fine without Zca?

367

Shouldn't the conflict be enforced at a higher level? Otherwise you're silently making +c,+zcmt resolve to +c here.

llvm/lib/Target/RISCV/RISCVInstrInfoZc.td

19 ↗

(On Diff #463097)

Blank line

20 ↗

(On Diff #463097)

This should line up?

28 ↗

(On Diff #463097)

I don't think we want this? cm.jt sym isn't a meaningful thing.

llvm/lib/Target/RISCV/RISCVSystemOperands.td

383

Capitalise (and maybe Jump Vector Table, assuming that's what jvt stands for? these headings come from the priv spec but that's woefully outdated for all the new asciidoc extensions...)

[VincentWu]

• update the document
• Throw an error when +c and +zcmt are both specified
• test CSR

llvm/lib/Target/RISCV/RISCV.td

366

the spec says:

The Zcmt extension depends on the Zca and Zicsr extensions.

I think maybe zcmt needs to be aligned using c.nop? That might be one of the reasons why zcmt required zca.