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

Added Instruction Description #1603

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AFOliveira
Copy link
Contributor

Hello!

This PR introduces a continuation of the approach to instruction-per-page documentation for RISC-V that has been followed by newer extensions like "B", Vector crypto and Scalar crypto. We are actively trying to improve the process of documentation generation and believe that aligning the base ISA with these extensions would greatly increase the manual. We recognize that there might be things to improve and that we are fully open to address any feedback.

Key points:

  • Enhanced instruction description structure:

    • Synopsis
    • Mnemonic
    • Encoding - with attributes
    • Description
    • Arguments
    • Operation (SAIL Code - retrieved from the SAIL repository)
    • Included in - (Extensions)
  • Automated documentation generation:
    As discussed in Enhance RISC-V Instruction Documentation #1590 we are also trying to achieve a nice automation process based on existing repos and minimizing our impact as we possible can. Please take note that, in order to achieve the most of this layout, we refined some of the SAIL Code by hand.

Current status:

  • Proof of concept covering a subset of RISC-V instructions (RV32I)
  • Example output available in the RISC-V instructions PDF riscv-unprivileged.pdf

There is a possibility here to impact the 200+ instructions in the manual that aren't still in this format if this approach is welcomed.

We welcome feedback on the approach, output quality, and potential modifications to existing repositories to support this effort.

Related issues: #1470, #1369, #1253, #1396, #1567

Co-authored-by: Afonso Oliveira  <[email protected]>
Co-authored-by: Alfredo Rodrigues <[email protected]>
@wmat
Copy link
Collaborator

wmat commented Aug 16, 2024

Where is src/instrs/instr-encoding-rv_i.adoc coming from? Was it generated somehow or manually created? What version of the SAIL model is the SAIL code from?

@AFOliveira
Copy link
Contributor Author

src/instrs/instr-encoding-rv_i.adoc is mostly automatically generated. The only part with a manual revision/creation is SAIL code.

This is branch with the latest version https://github.com/foss-for-synopsys-dwc-arc-processors/riscv-opcodes/tree/SynopsisDescription (if anynone wants to try please stick to "python3 parse.py rv_i -asciidoc", since the branch is still a WiP).

SAIL model is being directly parsed (both automatically and by hand) from the upstream repository.

@wmat
Copy link
Collaborator

wmat commented Aug 16, 2024

Would it not be better to have a single appendix of all instructions rather than listing them at the end of a chapter?

@wmat
Copy link
Collaborator

wmat commented Aug 16, 2024

Or perhaps even a separate document of all instuctions?

@psherman42
Copy link

psherman42 commented Aug 16, 2024

This is very nice, keeping each instruction on its own page, with writing brief enough to stay only within that one page.

It makes the ToC horrendously long, as well as breaking up continuity and flow of the rest of the Specification document.

This nice presentation seems much more fitting for another volume, may I suggest "RISC-V Assembly Lamguage Programming", following the lead of Lance Leventhal's masterpiece, "6502 Assembly Language Programming"? If you remember it, the layout and presentation is nearly the same as proposed by this PR.

@psherman42
Copy link

image

@abrodkin
Copy link

Or perhaps even a separate document of all instuctions?

That might very well be a separate document, but then what will be a distinction between what we already have in the ISA manual and that separate "index"? Note, that recently added extensions already have something like that. See, for example:

  • 33.15. Vector Mask Instructions
  • 34.4. Instructions (crypto instructions)
  • 35.2. Extensions Overview (vector crypto)

I.e. will we remove these instruction descriptions from the ISA manual and keep there only general wording about each extension?
Just for the record, our idea was to improve the existing ISA manual with a uniform presentation of all instructions.

@abrodkin
Copy link

This is very nice, keeping each instruction on its own page, with writing brief enough to stay only within that one page.

It makes the ToC horrendously long, as well as breaking up continuity and flow of the rest of the Specification document.

This nice presentation seems much more fitting for another volume, may I suggest "RISC-V Assembly Lamguage Programming", following the lead of Lance Leventhal's masterpiece, "6502 Assembly Language Programming"? If you remember it, the layout and presentation is nearly the same as proposed by this PR.

Well, there's already "RISC-V Assembly Programmer's Manual", see https://github.com/riscv-non-isa/riscv-asm-manual/blob/main/riscv-asm.md. And frankly speaking I'd rather consider merging it with the ISA manual as otherwise the ISA manual becomes a bit incomplete. In a sense that as a mere mortal software developer I would go check the ISA manual in a hope to get my questions answered fully, while in reality the information of my interest is spread among many documents:

  • The RISC-V Instruction Set Manual
    • Privileged &
    • Non-privileged
  • RISC-V Assembly Programmer's Manual
  • RISC-V ELF psABI Document
  • etc etc

I.e. I see value in modularity but we're also getting a problem of fragmentation, ambiguity and incompleteness (as it looks like from the first glance).

@psherman42
Copy link

psherman42 commented Aug 16, 2024

Not quite the same. Palmer's manual pointed out above is excellent coverage of best practices. It includes description of all relevant the directives in great detail, and lists all of the pseudoinstructions. Yet it doesn't include any mention of any of the real instruction, no summary and no detail.

That's where this comprehensive instruction list fits in -- following Leventhal. A separate thing to be a nice little pocket reference (for example, see Rick Aster's "Professional SAS Programmer's Pocket Reference").

It could/would even be given a coveted place as a riscv-isa-manual as opposed to being contribution simply as riscv-non-isa.

@AFOliveira
Copy link
Contributor Author

While there are differences between the ASM manual and this list, I believe there's still value in consolidating the information rather than dispersing it. Given that the instructions are almost entirely automatically generated, maintenance would be minimal in the event of any changes in the ISA.

IMO incorporating the instructions as proposed in the PR would align with the existing standard, as @abrodkin pointed out, thus minimizing changes to the manual. We remain highly receptive to feedback and I'm not opposed to a comprehensive index of all instructions. However, I would suggest organizing that index by extensions as well. This approach would facilitate quick reference to see which instructions each extension adds, although we'd need to address the repeated instructions across extensions.

@psherman42
Copy link

psherman42 commented Aug 16, 2024

These are even more good reasons for maintaining that comprehensive instruction list in a separate place. Multiple index lists (one by alphabetical, another by extension) makes even greater length and adds to the "confusion of too many things to look at" when looking at the Unprivigedged Specification to learn its big picture.

Being almost entirely automatically generated, so that maintenance would be minimal, is more of a non-human benefit; it adds little to human readability and overall comprehension. Thus, it's a minor point.

If there are too many lists, and too much detail, it is very hard to see the difference from one thing (instruction, in this case) to the next. That is how bugs and errors are introduced: flipping through to gather and group all desired instructions which might be part of scope of some desired revision, one might not catch them all--the missing ones would then be bugs later needing to be fixed. A cyclical and painful process.

The goal of a manual or specification document is to say as much as possible, using comparative terms and relational methods and explanations, with the least space used.

@anatoly-savchenkov
Copy link

In order to answer this question we need to agree on the use model for RISC-V ISA Manual. Is this a book for reading from first page to last page like a novel, or a Reference Manual for a hypothetical CPU which includes all latest versions of ratified RISC-V extensions, or a collection of adoc files grouped by extensions + their versions which IC manufactures can mix and match to create a Programmers' Reference Guide document for their respective chip.

Is the use model for RISC-V ISA Manual defined and documented somewhere?

@jalobaba
Copy link
Contributor

I'd like to ensure that this per-instruction documentation is just one view of a database of this information. I'd like to see other views including HTML and then also integration with other RISC-V ISA information like riscv-opcodes or some plan to integrate them together.

On a technical note, I'd like to see a list of possible exceptions that can be generated by an instruction added. See MIPS example.

image

@AFOliveira
Copy link
Contributor Author

Hello, @jalobaba.
It is just one view of the information of the database for sure.

views including HTML

We are using the make method of this repo so it definitely also generates HTML. If you want to check out how that would look, the easiest option would be to open the manual in it's current state and navigate to "BF16" extension since that is the style we are going for.

integration with other RISC-V ISA information like riscv-opcodes

Our source of information for all of this (except synopsis and description) is the riscv-opcodes repository. That's one of the main advantages of this whole approach since it almost only uses pre-existing information.

possible exceptions

Since we are parsing from riscv-opcodes, we can't automatically get this information, so it would have to be a manual adittion. We are already seing possibilities for this since the riscv-unified-db also has this information and it for sure is a great adittion.

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

Successfully merging this pull request may close these issues.

6 participants