Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding instruction type as a parameter for instruction definition #256

Open
AFOliveira opened this issue Nov 11, 2024 · 6 comments · May be fixed by #285
Open

Adding instruction type as a parameter for instruction definition #256

AFOliveira opened this issue Nov 11, 2024 · 6 comments · May be fixed by #285
Labels
enhancement New feature or request

Comments

@AFOliveira
Copy link
Collaborator

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.

@AFOliveira AFOliveira added the enhancement New feature or request label Nov 11, 2024
@james-ball-qualcomm james-ball-qualcomm changed the title Addding instruction type as a parameter for instruction definition Adding instruction type as a parameter for instruction definition Nov 12, 2024
@lenary
Copy link
Collaborator

lenary commented Nov 12, 2024

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
Copy link
Collaborator Author

AFOliveira commented Nov 12, 2024

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
Copy link
Collaborator

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: riscv/sail-riscv#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: ThinkOpenly/sail#40.

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

@lenary
Copy link
Collaborator

lenary commented Nov 12, 2024

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
Copy link
Collaborator

I think this goal fits nicely with #203. Using $ref/$inherits, we can, without a bunch of redundancy, add ABI formats. I would definitely support doing so.

@dhower-qc dhower-qc linked a pull request Nov 22, 2024 that will close this issue
@lenary lenary linked a pull request Dec 3, 2024 that will close this issue
@lenary
Copy link
Collaborator

lenary commented Dec 3, 2024

I left a comment on the PR which supersedes some of my opinions left here, and finds more room for compromise and hopefully specificity: #285 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants