add checks on slack bits #217
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Before this commit, one function called aop() was responsible for parsing arithmetic instructions both with and without immediate values. After this commit, aop() is responsible for instructions without immediate values. A new function aopi() takes over the arithmetic instructions with immediate values. Arithmetic instructions with/without immediate values have different kind of checks with respect to funct7 bits. Two separate functions are easier to read.
The corresponding changes are being made in #217
vm/src/rv32/parse.rs
Outdated
| assert_eq!(opcode(word), OPC_ALUI); | ||
| let res = match (funct3(word), funct7(word)) { | ||
| (0b000, _) => ADD, | ||
| (0b001, 0b0000000) => SLL, |
There was a problem hiding this comment.
We should document here with a comment that funct7 happens to capture the same set of bits as the imm[11:6] chunk that distinguishes the type of intermediate-based shift (slli vs. srli vs. srai).
Otherwise at first glance it looks like we're parsing them wrong since they don't include a funct7 chunk.
There was a problem hiding this comment.
Added comments.
There was a problem hiding this comment.
These weren't really the comments I was looking for. What was confusing wasn't the _ matching on the lines that use the full intermediate, what was confusing was the explicit matching for the shifts (since they don't use funct7 at all). If you can add (technicallly imm[11:5]) like above that'd be great.
vm/src/rv32/parse.rs
Outdated
| assert_eq!(opcode(word), OPC_ALUI); | ||
| let res = match (funct3(word), funct7(word)) { | ||
| (0b000, _) => ADD, | ||
| (0b001, 0b0000000) => SLL, |
There was a problem hiding this comment.
These weren't really the comments I was looking for. What was confusing wasn't the _ matching on the lines that use the full intermediate, what was confusing was the explicit matching for the shifts (since they don't use funct7 at all). If you can add (technicallly imm[11:5]) like above that'd be great.
yoichi-nexus
force-pushed
the
stricter-format-checks
branch
from
eab6f2c
to
352b76e
Compare
I did a few more git push'es to give better approximation in comment formulation. |
vm/src/rv32/parse.rs
Outdated
| (0b010, _ /* imm[11:5] can be any */) => SLT, | ||
| (0b011, _ /* imm[11:5] can be any */) => SLTU, | ||
| (0b100, _ /* imm[11:5] can be any */) => XOR, | ||
| (0b101, 0b0000000 /* shift amount is required to be small */) => SRL, |
There was a problem hiding this comment.
Also I'm confused by the "small" part, this value doesn't have anything to do with the shift amount, right? It's a fixed value to indicate the type of shift, I think.
There was a problem hiding this comment.
I can argue both ways, but I'll just say something else, like, SLLI requires special value here.
yoichi-nexus
force-pushed
the
stricter-format-checks
branch
from
352b76e
to
b32dfb5
Compare
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
The corresponding changes are being made in #217
* Spec initial pass * Remove translation * Update instruction list * Updating instruction layout table (I'm appending changes to this commit until the update completes.) * Update prose * Describe auipc instruction * Describe lui instruction * Fix typo * Describe unimp instruction * Describe fence instruction * Another pass on the prose * Add vm architecture drawio file with permission of Michel Abdalla * Update architecture diagram Just one input tape for private input for now. * Another pass * Review * Review: ecall and ebreak take rd register selector * Strictor bit pattern as in RISC-V The corresponding changes are being made in #217 * Explicitly describe the sizes of immediates * Opcode vs mnemonic changes * Rephrase sign/zero extension * lw doesn't perform any extension, zero or signed * More specific wording on which byte/half-word gets stored * Add missing angle brackets * Modify version numbers * Addressing review comments * Change version sequences in the roadmap
@michel-nexus noticed that the unused bits can be checked to be a specific value, like in RISC-V ISA. This PR implements this idea.