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

Enhance RISC-V Instruction Documentation #1590

Open
AFOliveira opened this issue Aug 8, 2024 · 9 comments
Open

Enhance RISC-V Instruction Documentation #1590

AFOliveira opened this issue Aug 8, 2024 · 9 comments

Comments

@AFOliveira
Copy link
Contributor

Building on issues #1470 #1369 #1253 #1396 #1567 and aligning with the RISC-V Unified Database efforts, I've made some tweaks that I believe could potentially improve the description of all instructions in the manual.
Currently, instruction description generally follows this structure:

Screenshot 2024-08-08 112122

IMO information is too condensed and this makes it a bit harder to understand, so I believe that the following example of the following standard is a much better alternative:

image

This approach clarifies instruction descriptions and can be efficiently generated using existing tools. By leveraging riscv-opcodes (particularly parse.py) and sail-riscv, I've automated the creation of instruction-per-page documentation. The process extracts encodings, mnemonics, and argument lists from riscv-opcodes, then obtains and post-processes SAIL code from sail-riscv for each instruction, maintaining readability and executability.

The main challenge with this approach lies in the SAIL code's structure. SAIL, written as conventional code, lacks certain conventions that would facilitate easier parsing and post-processing. While it's possible to work with the current SAIL repository, doing so would be time-consuming and less adaptable to future changes in riscv-sail/model. Implementing specific conventions could significantly streamline this process.

As an early stage work, this approach currently covers a subset of RISC-V instructions. The preliminary set of supported instructions can be found in this RISC-V instructions PDF.

The branch where this work can be checked is SAIL WiP, but please take it only as a Proof of Concept since it is not finished.

@wmat
Copy link
Collaborator

wmat commented Aug 8, 2024

Interesting effort. In looking at the result, some of the encoding diagrams are messed up with text overlaying the boundaries of the bit they're in, for example the BEQ instruction. What would be really useful is if you could generate two .adoc files for inclusion as an indices in the Unprivileged and Privileged manuals.

@AFOliveira
Copy link
Contributor Author

Thank you so much for your feedback! I'm aware of some existing bugs, including the one you've highlighted. This specific issue might be resolved as current encodings do:

image

Creating two-document support for the instructions index is definitely viable with this approach. I've made initial code changes, but I need to review their scalability across the entire ISA.

I can't upload the .asciidoc here so please check if these two small versions of the Unprivileged Instruction Index and Privileged Instruction Index correctly address your feedback.

@wmat
Copy link
Collaborator

wmat commented Aug 8, 2024

Those seem to satisfy the split. How are you getting the SAIL code, btw? I don't see any include statments in the code, nor do I see a SAIL index.

@AFOliveira
Copy link
Contributor Author

@wmat sorry for the delay.
The current approach for obtaining SAIL code assumes the root folders for riscv-opcodes and sail-riscv repositories are the same. In the parse.py file, the make_asciidoc_sail function specifically references one directory of the SAIL repo.

We're referencing a specific file in this version due to variable naming in SAIL code. One of the main flaws of this approach is that there doesn't seem to be an appropriate variable naming convention that would allow easy correlation between both repositories.

While it's possible to correlate all files this way, doing so might make the code less flexible and require constant updates when new extensions are added.

I believe that minimal changes on both sail-riscv and riscv-opcodes repositories could hugely enhance this approach for documentation. However, since those are functional repos, I prefer to ask for support here to check if minimally changing those might be a possibility.

@edolnx
Copy link

edolnx commented Aug 26, 2024

Adding references to the SAIL repoistory for every instruction/CSR is something I am working towards as part of the Unified DB project. It's not going to be perfect, but it will make it easier to find and address those as the Golden Model project break apart more of their code base.

@wmat
Copy link
Collaborator

wmat commented Aug 26, 2024 via email

@edolnx
Copy link

edolnx commented Aug 26, 2024

I'd also like to expand upon this layout as a basis for all instructions moving forward. Several of us are working on a goal to have these pages generated from the Unified DB, and there are cross-extension additions that I'm trying to address. Especially as we move forward with the model of One Single Repo for all the ISA Specification where all new extensions are created in a branch, I think it's critical that we reduce the scope of changes otherwise the merge process will be excruciating. To that point, there are extensions that modify the behavior of existing instructions/CSRs/extensions. Ideally we should be able to "top insert" or "bottom append" information to a Documentation Block to the affected instructions/CSRs/extensions from a new instruction/CSR/extension via the Unified DB. But this can only happen if we have a well defined set of Blocks for each page so that they can be targeted for "top insert" or "bottom append". What is listed in this design is a good starting point, but we should probably look a little wider and ensure we have all the necessary blocks well defined, even if they are blank, in every page.

@edolnx
Copy link

edolnx commented Aug 26, 2024

On Mon, Aug 26, 2024 at 1:11 PM Carl Perry @.> wrote: Adding references to the SAIL repoistory for every instruction/CSR is something I am working towards as part of the Unified DB project. It's not going to be perfect, but it will make it easier to find and address those as the Golden Model project break apart more of their code base.
This is exactly what the asciidoctor-sail extension is for. https://github.com/Alasdair/asciidoctor-sail For the record, I'm trying to document the how to make it easier for extension writers to reference SAIL.

— Reply to this email directly, view it on GitHub <#1590 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAN6ZBSRSX57GLZ5MESVPLZTNOUTAVCNFSM6AAAAABMGNCIJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJQGY3TONJYGQ . You are receiving this because you were mentioned.Message ID: @.
>

I was unaware of the existence of this, thanks for the info. I look forward to reading more about it and how to integrate it.

@AFOliveira
Copy link
Contributor Author

To that point, there are extensions that modify the behavior of existing instructions/CSRs/extensions. Ideally we should be able to "top insert" or "bottom append"

I believe this for sure have space there, as I referenced earlier this is just a base in which things can be added or removed. Talking with @dhower-qc we already got to the conclusion that if the description is good enough, there might not be a point in having an argument list, and therefore we removed it in the yaml version. I have been working in this yaml version which is on riscv-software-src/riscv-unified-db/pull/21 that generates the same thing that is here but in .yaml. I think we can definitely add that parameter both here and there. I think I will add that to the #1603
and reference the new comments here.

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

No branches or pull requests

3 participants