riscv-software-src / riscv-unified-db

Machine-readable database of the RISC-V specification, and tools to generate various views
Other
5 stars 5 forks source link

Improve RISC-V pseudo-instructions #12

Open james-ball-qualcomm opened 1 month ago

james-ball-qualcomm commented 1 month ago

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 commented 4 weeks ago

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

https://github.com/riscv-software-src/riscv-unified-db/blob/1ad462fa6166b6c7206ceb8180a5f8ca28aed1be/arch/inst/B/add.uw.yaml#L25-L27

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.

james-ball-qualcomm commented 4 weeks ago

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

dhower-qc commented 4 weeks ago

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 commented 4 weeks ago

Either solution looks good to me.

AFOliveira commented 3 weeks ago

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 commented 3 weeks ago

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.