search

riscv-software-src / riscv-unified-db

Machine-readable database of the RISC-V specification, and tools to generate various views
Other
17 stars 14 forks source link

Adding instruction type as a parameter for instruction definition #256

Open AFOliveira opened 5 days ago

AFOliveira commented 5 days ago

Is your feature request related to a problem? Please describe. Describing instructions can be tricky, and by defining which type they belong to, we know that they must fit into a certain characteristics, like enconding.

This can help in a ton of ways, e.g. allow for the regression test to validate if the bits in the registers are correct, displaying the information in the final html/pdf since it is ratified information and allow more complete information in registers (i.e. we know that on C extension rd/rs1 can never be 0),

Describe the solution you'd like Add the type parameter to instruction description. Also, add all the possible types (e.g. R-Type, I-type ...) and what characteristics this type has, I am not too sure on how to do this second part? Does json allow it?

Aditionally, we can also use it on the regression test.

lenary commented 4 days ago

This is also required for the psABI, which defines relocations in terms of instruction types rather than specific instructions.

I would like to see instruction type adopted by the unified database not just as a textual label but in a more structured form that can be used to ensure that instruction encodings do indeed fit their type. This could even be done definitionally, by making the encoding information for an instruction encode/decode/inherit fields from the instruction's type, rather than just pulling them out of a bitvector. This might avoid some redundancy, and should make it easier to produce diagrams about specific instructions that are similar to those in the ISA specification today.

AFOliveira commented 4 days ago

I would like to see instruction type adopted by the unified database not just as a textual label but in a more structured form that can be used to ensure that instruction encodings do indeed fit their type.

This was what I was trying to say when I meant it would be helpful for the regression test. It will help on the UDB testing proccess.

I will add the text labels ASAP, so that we can work out on how to perform the tests.

ThinkOpenly commented 4 days ago

This is also required for the psABI, which defines relocations in terms of instruction types rather than specific instructions.

I would like to see instruction type adopted by the unified database not just as a textual label but in a more structured form that can be used to ensure that instruction encodings do indeed fit their type. This could even be done definitionally, by making the encoding information for an instruction encode/decode/inherit fields from the instruction's type, rather than just pulling them out of a bitvector. This might avoid some redundancy, and should make it easier to produce diagrams about specific instructions that are similar to those in the ISA specification today.

The Sail model is currently of no help in this regard, obviously. FWIW (perhaps nothing), I took at stab at implementing this within the Sail model itself: https://github.com/riscv/sail-riscv/pull/546. In hopeful anticipation of acceptance in some form, a RISC-V Mentee had started on changes to the "Sail to JSON" out-of-tree Sail parser backend to pull the respective instruction formats into JSON: https://github.com/ThinkOpenly/sail/pull/40.

Let's just say reactions to the PR were tepid, and I haven't pushed for more discussion yet.

lenary commented 4 days ago

I sort-of understand the view that instruction types/formats are currently a rule of thumb in the ISA spec, but maybe this is an opportunity to push for more rigorous definitions of instruction types in a machine-readable way?

I do understand that this makes the sail-only model harder to understand, and that hardware engineers largely only care about the raw encoding bits, but I think modelling concepts the existing ISA talks about, and that we've written some other specifications (the psABI) in terms of is a good thing.

It really depends on how far one expects these specifications to cross/model the hardware/software boundary. If a specification is only for hardware, instruction types are harder to justify.

dhower-qc commented 3 days ago

I think this goal fits nicely with https://github.com/riscv-software-src/riscv-unified-db/issues/203. Using $ref/$inherits, we can, without a bunch of redundancy, add ABI formats. I would definitely support doing so.