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

No mention of jr pseudo-instruction #1253

Closed
dzaima opened this issue Mar 9, 2024 · 9 comments · Fixed by #1555
Closed

No mention of jr pseudo-instruction #1253

dzaima opened this issue Mar 9, 2024 · 9 comments · Fixed by #1555

Comments

@dzaima
Copy link

dzaima commented Mar 9, 2024

The manual mentions jr twice - in the LR/SC example and for cm.jt (and gcc and clang -S emit it for indirect calls), but I can't find any description of it in the manual. It should be mentioned as a pseudoinstruction for jalr x0, 0(rs1).

@aswaterman
Copy link
Member

See the RISC-V Assembly Programmer's Manual. We don't see fit to duplicate that information in the ISA spec.

https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md

@abrodkin
Copy link

See the RISC-V Assembly Programmer's Manual. We don't see fit to duplicate that information in the ISA spec.

https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md

I'm wondering why pseudoinstructions don't fit well in the ISA manual. For most of the mere mortals dealing with RISC-V assembly (either reading it or writing) pseudoinstructions don't look any different compared to "normal" instructions. And given pseudoinstructions don't depend on ABI or something else, it would be so much more convenient to just have them listed where the rest of instructions are described. I.e. copy or even move https://github.com/riscv-non-isa/riscv-asm-manual/blob/main/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions into this ISA manual.

@aswaterman
Copy link
Member

I agree that the current situation, where the ISA spec uses a small number of pseudoinstructions but does not define them, makes it hard on readers. Solutions include:

  • Don't use pseudoinstructions in the ISA manual. (This is a poor choice, IMO, since the pseudoinstructions are generally easier to read, and since they convey proper style.)
  • Have a short section that defines only the pseudoinstructions that the manual uses. (This avoids duplicating so much material from the ASM manual.)
  • Merge the ASM manual into back into the ISA spec at some point in the future.

@dzaima
Copy link
Author

dzaima commented Jul 17, 2024

For jr in particular, what I found odd is that, just above jalr, there is:

Plain unconditional jumps (assembler pseudoinstruction J) are encoded as a JAL with rd=x0.

which does mention a different pseudoinstruction, from which I expected that related pseudoinstructions would be mentioned as they come up.

@abrodkin
Copy link

Merge the ASM manual into back into the ISA spec at some point in the future.

Even though I would prefer the 3rd option of those listed above, I'm not sure the entire ASM manual could be easily merged in the ISA manual because of way so many ABIs. I.e. current ASM manual (https://github.com/riscv-non-isa/riscv-asm-manual/blob/main/riscv-asm.md) refers to some not clearly defined ABI, but in the ISA manual we'll need to be clear and precise, which might mean addressing at least most widely used ABIs.

@kasanovic
Copy link
Collaborator

I remember some other ISA manuals list the pseudo-instructions in the "page-per-instruction" reference section, calling out that these are specialized variants of the other instruction.

@aswaterman
Copy link
Member

aswaterman commented Jul 24, 2024

We discussed this in the Architecture Review Committee meeting today, and we decided that the ISA spec can normatively define some pseudoinstructions, particularly no-brainers like MV. It's OK if there's partial overlap with the ASM manual, though the ASM manual is beholden to respecting any normative pseudoinstruction definitions in the ISA spec.

To resolve this specific issue, we will define the JR and RET pseudoinstructions just after the definition of JALR. As @dzaima points out, this is symmetric with J/JAL.

@aswaterman aswaterman reopened this Jul 24, 2024
aswaterman added a commit that referenced this issue Jul 24, 2024
Symmetric with defining J pseudoinstruction earlier in the same section.

Resolves #1253
aswaterman added a commit that referenced this issue Jul 24, 2024
Symmetric with defining J pseudoinstruction earlier in the same section.

Resolves #1253
@abrodkin
Copy link

So is my understanding correct that there's no plan for adding all existing pseudoinstructions in the ISA manual, instead we'll act on the case-by-case basis addressing requests for adding particular pseudoinsturctions? Sorry for bringing this up again, but my worry is that now the ISA manual will look inconsistent: some pseudoinstructions are there, while some are not. What's actually holding us from adding all the pseudoinstructions while we're at it?

@kasanovic
Copy link
Collaborator

The ISA manual should list all the standard pseudoinstructions that expand 1:1 to machine instructions, but the ISA manual should not list pseudoinstructions that assembler can expand into more than one machine instruction. Assemblers may also define additional pseudoinstructions that are 1:1 for some reason, but the ISA manual should only list the canonical ones expected to be supported by all assemblers.

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 a pull request may close this issue.

4 participants