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

Improve RISC-V pseudo-instructions #12

Open
james-ball-qualcomm opened this issue Aug 12, 2024 · 6 comments
Open

Improve RISC-V pseudo-instructions #12

james-ball-qualcomm opened this issue Aug 12, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@james-ball-qualcomm
Copy link
Collaborator

Can we add defined pseudo-instructions to riscv-unified-db?

Here's a port of a post by Ved (link):
"ARC discussed a question about the location of pseudo- instructions with respect to the ISA manual and concluded that a list of pseudo-instructions should be included in the ISA manual. A PR is planned."

@dhower-qc
Copy link
Collaborator

There is a field for adding pseudo instructions to an instruction definition. For example, in add.uw.yaml from the B extension:

pseudoinstructions:
when: rs2 == 0
to: zext.w xd, xs1

It's not super thought-out, though, and could probably use some tweaking. Getting some more examples of pseudo instructions we want to define would help.

@dhower-qc dhower-qc added the enhancement New feature or request label Aug 14, 2024
@james-ball-qualcomm
Copy link
Collaborator Author

Maybe we can get a full list from someone with experience with the RISC-V toolchains (like Ana)?

@dhower-qc
Copy link
Collaborator

Here's an interesting case:

RDCYCLE is a pseudo instruction defined by Zicntr, but it's an alias of csrrs from Zicsr. The current schema that adds the pseudo instruction under the real instruction isn't a great match since csrrs says it belongs to Zicsr, not Zicntr.

A few possibilities:

  1. The existing pseudoinstructions field can add a definedBy key, similar to how CSR fields can have their own definedBy that is different than the parent CSR.
  2. Pseudo-instructions could get their own definition file.

I'm inclined to prefer 1. since it matches what we're doing for CSRs/CSR fields, but that's not a strong preference.

@james-ball-qualcomm
Copy link
Collaborator Author

Either solution looks good to me.

@dhower-qc dhower-qc changed the title Add RISC-V pseudo-instructions Improve RISC-V pseudo-instructions Aug 19, 2024
@AFOliveira
Copy link
Collaborator

RDCYCLE is a pseudo instruction defined by Zicntr, but it's an alias of csrrs from Zicsr. The current schema that adds the pseudo instruction under the real instruction isn't a great match since csrrs says it belongs to Zicsr, not Zicntr.

Please correct me if I misunderstood all of this, but doesn't this mean that the riscv-opcodes is so far wrong in defining this pseudo-instruction(and many more from what I have checked)? Since the instruction is defined as

$pseudo_op rv_zicsr::csrrs rdcycle rd 19..15=0 31..20=0xC00 14..12=2 6..2=0x1C 1..0=3

inside the rv_zicsr file.

It seems even a bit more tricky than this since there isn't even a rv_zicntr file, while the spec definitely has them as two different extensions:
image

@dhower-qc
Copy link
Collaborator

Please correct me if I misunderstood all of this, but doesn't this mean that the riscv-opcodes is so far wrong in defining this pseudo-instruction(and many more from what I have checked)? Since the instruction is defined as

$pseudo_op rv_zicsr::csrrs rdcycle rd 19..15=0 31..20=0xC00 14..12=2 6..2=0x1C 1..0=3

inside the rv_zicsr file.

Yes, I believe riscv-opcodes is incorrect in this case. In general, I've found that there is quite a bit related to Zicsr/Zicntr/Zihpm that is not correct in the spec/riscv-opcodes, etc. Presumably, this is because those extensions were bolted on as an afterthought; originally, everything was defined in the base architecture.

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

No branches or pull requests

3 participants