-
Notifications
You must be signed in to change notification settings - Fork 309
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
Migration to new file-naming and instruction category scheme #106
Conversation
- the ordering in these files have changed to preserve the order in the latex- tables - also ecall and ebreak has been moved to rv_i instead of keeping them in 'systems' file.
- the files are simply renamed
- the re-ordering of opcodes s necessary to preserve latex-table order
- all changes involve re-ordering to preserve order in latex-tables
- all changes involve re-ordering to preserve order in latex-tables
- the previous opcodes used ignore to define immediate fields instead of assigning arg names to it. This is made it difficult to parse and decode the instructions. - this commit assigns unique names to immediate fields in accordance to what has been done elsewhere. Note these names hold no correspondence to the spec and are defined here purely to ease decoding - This commit also splits the instructions which depend on F/D/Q in to their respective files as per new naming convention - c.nop encoding has been changed to include hints as well.
- involves renaming and minor reordering to preserve latex tables
- involves only renaming
- renaming files and reordering to preserve latex-table order
- involves renaming of files
- aliases have been revised to use $pseudo_op syntax - split the instructions into multiple files as per new file naming policy - some pseudo ops depend on unratified instructions.
- split instructions as per new file naming policy - here the 32-bit ops are considered pseudo_ops of the 64-bit equivalents as they only differ in one-bit.
- renamed the variable imm12 to csr to match the latex-table entries.
- all prefetch insructions have been made appropriate pseudo_ops of ori instruction
- includes zbp and zbpbo sub extensions as well
- split instruction based on new file naming policy - import instructions present in other ratified and unratified extensions - pseudo ops from zbp and zbb defined
- split instructions as per new file naming policy - move all instructions to unratified directory - includes rv[64|32]_zbp[bo] (missed in previous commit while migrating P-extension)
- significant restructuring of opcodes into files as per new file naming policy
- pseudo ops cannot be imported. The pseudo_op syntax itself should be used where applicable.
- the python file is well commented - the README provides a brief overview of how the python script works and the various artifacts it can generate
- this changes the imports in zk[ns] - this name is also what spike uses for now. - This fix may come-back later when zbp and zbkx reconcile on a common naming scheme for these instructions.
The custom variants are generic instruction formats for the RoCC (Rocket Custom Coprocessor) extension, where bits 14:12 indicate the presence of operands The decode logic in Rocket, Boom, and a few other cores rely on separate bit-patterns being emitted for each variant: |
For the most part, these are aliases: the instructions were renamed during the standardization process, but we chose to retain the old names as aliases for compatibility. See, for example, the note at the beginning of this section: https://github.com/riscv/riscv-v-spec/blob/d659c8d91b6ed2a3be41e1f58f868c20f0f161b9/v-spec.adoc#152-vector-count-population-in-mask-vcpopm |
@aswaterman ok will make those changes for the rvv-pseudo ops. Will rename the file as rv_v_aliases and use the regarding, the custom opcodes - I understand they are RoCC compliant but do they need to reside here since RoCC is not a RVI adopted/supported spec (hence their are niether ratified/unratified). If they do need to sit here: then they are not parse-able in their current format and will need changes. For example:
is overspecified - rs2 and imm12 have overlapping bits and the parser will complain with an error. |
@aswaterman changes for vector aliases done and also fixed the github actions yaml. I think its ready to merge. I will come back with the custom opcodes fix in a separate PR |
@neelgala @a0u My preference is to delete the RoCC instructions from this repo. Their presence is a historical artifact. These days, we'd reject PRs to add other folks' custom instructions, and so we shouldn't be privileging our Berkeley tech. (It's OK that Rocket's/BOOM's |
@aswaterman done. removed custom opcodes. |
Thanks. I will review the rest of this PR shortly. |
@aswaterman just setting a reminder on this. Let me know if I can do something to ease the review on this ? Also I request : if you could merge this PR before other PRs so I avoid re-factoring those again here. |
@aswaterman I've made the required changes. Also added the spinalhdl support currently on master. I have locally checked the conflicts and you should be picking changes made on my PR (the parse_opcodes file goes away and the make-targets are redone based on new cli). I am unable to resolve these at my end - let me know if I can help fix it. |
@neelgala Merged, but I just noticed you deleted support for Go, i.e., |
will raise a PR asap. |
This PR is motivated by the discussion in issue #100 and addresses the concerns raised there.
It is a backwards incompatible PR, and any one depending on this repo or its artifacts will need to
bump their environment (with minimal effort) after this PR is merged.
I have tried my best to split the PR in smaller commits to easier review. However, the individual commits themselves if checked out will be broken.
Brief summary of changes:
This PR can also close issues: #8, #19, #32, #61, #89, #100
Changes in Artifacts that need to be reviewed:
encoding.h :
done earlier. This is because there exist pseudo-ops which have the same mnemonics as the regular
instruction but differ is just few bits. Eg. slli in RV32 and RV64 have the same mnemonic is both,
however in RV32 slli is treated as a pseudo_op of RV64::slli with bit 25 set to 0. Thus it is
impossible to include both variants of the instruction. Also spike doesn't make use the
pseudo-ops.
encodings as well. Previously only
c.nop 0
was only considered as legal instruction. Now, theimmediate value rangin from 0-31 will be considered as a
c.nop
intruction which is what thespec suggests as well
latex-tables:
drafted and hard-coded in a very wierd fashion. In this PR, all the latex tables have 33
columns: 32 columns for the encoding (1bit per column) and one column for the instruction name.
This does not affect rendering of the tables in the pdf - manually verified.
the change is mostly with the ordering of instructions. Since, SLLI, SRAI and SRLI are
pseudo_ops in rv32_i they are processed at the end and thus appear at the end of the table
instead of after ANDI. Same argument goes for FENCE.TSO and PAUSE insrtuctions.
rv_d_zfh
andrv_q_zfh
extensions according to the new convention.NEED HELP
Need help in understanding the following:
I am not sure how the opcodes-custom instructions are meant to be interpreted ? Are all thosevariants of custom0 required ? Can we instead just have 4 entries
Though, I am still not sure what value this brings in this project !The instructions in opcodes-rvv-pseudo don't seem to be actual pseudo ops. These instructions onlyhave a different name but have the exact same encoding as far as I can see (no hardwiring of like
other pseudo ops). Do we need them ?