Add unratified Zimop extension #1467
Conversation
|
@ved-rivos I’d like to refactor this to reduce the number of instructions perceived by Spike. The idea is that we’d have a single instruction for each of mop.r, mop.rr, and c.mop. A switch statement can perform the dispatch on the relevant opcode bits. This presumably requires defining three new pseudoinstruction within riscv-opcodes. |
|
@aswaterman that would work. Will need a wider match/mask for each opcode group in opcodes. Will not need a switch I guess - they all have the same action. I am not sure how I can get the disassembler to show separate mnemonics - will look through it but if there is an example of such a pattern already please give me a pointer. |
|
@ved-rivos Right. I guess I meant we'd need a switch when implementing e.g. Zicfiss. As it stands, the switch is unnecessary. I think your disassembler implementation is fine as-is. (riscv-opcodes will have both the current opcodes and the new alias that subsumes them all, so the disassembler code will work without change.) |
|
@aswaterman - the PR is updated to use the mop.r.N and mop.rr.N pseudo-instructions. |
riscv/riscv.mk.in
Outdated
| @@ -1395,6 +1395,10 @@ riscv_insn_ext_zvksh = \ | |||
| vsm3c_vi \ | |||
| vsm3me_vv \ | |||
|
|
|||
| riscv_insn_ext_zimop = \ | |||
| mop_r_N \ | |||
There was a problem hiding this comment.
This file uses tabs--probably for bad reasons--but please be consistent with the surrounding spacing.
There was a problem hiding this comment.
Fixed it. My editor was setup to replace tabs with space.
riscv/riscv.mk.in
Outdated
| @@ -1431,6 +1435,7 @@ riscv_insn_list = \ | |||
| $(riscv_insn_priv) \ | |||
| $(riscv_insn_smrnmi) \ | |||
| $(riscv_insn_svinval) \ | |||
| $(riscv_insn_ext_zimop) \ | |||
There was a problem hiding this comment.
Tabbing comment applies here, too.
There was a problem hiding this comment.
Thank you for humoring my suggestion, @ved-rivos. I'll hold off on merging this (and #1468) until riscv/riscv-opcodes#194 is sorted out, but otherwise, LGTM!
ved-rivos
force-pushed
the
zimop
branch
2 times, most recently
from
a83c411
to
0bfdb58
Compare
|
@aswaterman riscv/riscv-opcodes#194 is sorted out, |
|
@ved-rivos can you clean up this PR so that it passes regression tests? Note that for Spike we require every commit in the PR to pass individually, so the problem might be that you need to reorganize your commits (or possibly squash some of them together) so that each commit is valid. |
|
@aswaterman - I had to squash the commits and it passes the checks now. I did not realize the system checks each commit to be buildable and testable. I will keep that in mind for future for organizing the commits. |
|
I am interested in zimop extension. Is there an update to this PR? |
|
@ved-rivos sorry for letting this lapse. Can you rebase (possibly updating encoding.h once again)? @YenHaoChen I do want to merge this promptly; it just happened to fall by the wayside. |
|
@aswaterman @YenHaoChen - I will rebase, run tests, and try to push it in today. |
Specification: https://github.com/riscv/riscv-isa-manual/blob/main/src/zimop.adoc
Sail ref: riscv/sail-riscv#309
Arch tests: riscv-non-isa/riscv-arch-test#387