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

Does Sail or JSON need 32bit-specific encodings? #19

Open
ThinkOpenly opened this issue May 1, 2024 · 8 comments
Open

Does Sail or JSON need 32bit-specific encodings? #19

ThinkOpenly opened this issue May 1, 2024 · 8 comments

Comments

@ThinkOpenly
Copy link
Owner

Some instructions have different encodings between RV32 and RV64.

From the RISC-V ISA Specification (20191213), Chapter 24, "RV32I Base Instruction Set":
image

From the RISC-V ISA Specification (20191213), Chapter 24, "RV64I Base Instruction Set (in addition to RV32I)":
image

Note the first field is 7 and 6 bits, respectively, and the 2nd field ("shamt") is 5 and 6 bits, respectively.

Anecdotally, the "binutils" project treats these differently (from "include/opcode/riscv-opc.h"):

#define MASK_SLLI       0xfc00707f
#define MASK_SLLI_RV32  0xfe00707f

#define MASK_SRAI       0xfc00707f
#define MASK_SRAI_RV32  0xfe00707f

#define MASK_SRLI       0xfc00707f
#define MASK_SRLI_RV32  0xfe00707f

From a practical standpoint, however, it's not clear that a distinction is necessary. The "shamt" field in these cases is located such that the if "shamt" was extended by another bit on the left, it would be the lowest order bit of the first field, and for "SLLI", "SRAI", and "SRLI", that bit is always zero:
image

@vishisht-dubey
Copy link

Hello @ThinkOpenly is this issue for discussion only or is it really a problem can you tell a little bit more

@ThinkOpenly
Copy link
Owner Author

While poking around in binutils, these instructions turned up as "32-bit specific" encodings:

#define MASK_SLLI_RV32  0xfe00707f
#define MASK_SRLI_RV32  0xfe00707f
#define MASK_SRAI_RV32  0xfe00707f

The Sail code hides this distinction slightly, at least enough for the project to currently ignore it:

mapping clause encdec = SHIFTIOP(shamt, rs1, rd, RISCV_SLLI) <-> 0b000000 @ shamt @ rs1 @ 0b001 @ rd @ 0b0010011 if sizeof(xlen) == 64 | shamt[5] == bitzero
mapping clause encdec = SHIFTIOP(shamt, rs1, rd, RISCV_SRLI) <-> 0b000000 @ shamt @ rs1 @ 0b101 @ rd @ 0b0010011 if sizeof(xlen) == 64 | shamt[5] == bitzero
mapping clause encdec = SHIFTIOP(shamt, rs1, rd, RISCV_SRAI) <-> 0b010000 @ shamt @ rs1 @ 0b101 @ rd @ 0b0010011 if sizeof(xlen) == 64 | shamt[5] == bitzero

In the above representation, shamt is always 6 bits, as can be seen here:

 * Note: For RV32, the decoder ensures that shamt[5] = 0.
 */
union clause ast = SHIFTIOP : (bits(6), regidx, regidx, sop)

(The "Note" in the comment there, is certainly relevant.)

However, this is arguably different than what is specified in the current text ISA (20191213), as can be seen in the image at the end of the opening comment in this issue. There, shamt is 5 bits for RV32.

The question to be resolved here is if it really matters. Pedantically, it matters: if the JSON should match the text ISA spec, shamt in RV32 is 5 bits. In reality, though, that 6th bit that would presumably be at position 25 in RV32 representations as it is in the respective RV64 representations, is always zero for these instructions (SLLI, SRLI, SRAI), as can also be seen in the same image above. So, for encoding and decoding, it has no practical impact.

So, practically, it can be ignored. The disadvantage to ignoring, though, is that the JSON cannot be used to generate the content for projects like binutils (or binutils would have to similarly ignore this case).

@vishisht-dubey
Copy link

While poking around in binutils, these instructions turned up as "32-bit specific" encodings:

#define MASK_SLLI_RV32  0xfe00707f
#define MASK_SRLI_RV32  0xfe00707f
#define MASK_SRAI_RV32  0xfe00707f

The Sail code hides this distinction slightly, at least enough for the project to currently ignore it:

mapping clause encdec = SHIFTIOP(shamt, rs1, rd, RISCV_SLLI) <-> 0b000000 @ shamt @ rs1 @ 0b001 @ rd @ 0b0010011 if sizeof(xlen) == 64 | shamt[5] == bitzero
mapping clause encdec = SHIFTIOP(shamt, rs1, rd, RISCV_SRLI) <-> 0b000000 @ shamt @ rs1 @ 0b101 @ rd @ 0b0010011 if sizeof(xlen) == 64 | shamt[5] == bitzero
mapping clause encdec = SHIFTIOP(shamt, rs1, rd, RISCV_SRAI) <-> 0b010000 @ shamt @ rs1 @ 0b101 @ rd @ 0b0010011 if sizeof(xlen) == 64 | shamt[5] == bitzero

In the above representation, shamt is always 6 bits, as can be seen here:

 * Note: For RV32, the decoder ensures that shamt[5] = 0.
 */
union clause ast = SHIFTIOP : (bits(6), regidx, regidx, sop)

(The "Note" in the comment there, is certainly relevant.)

However, this is arguably different than what is specified in the current text ISA (20191213), as can be seen in the image at the end of the opening comment in this issue. There, shamt is 5 bits for RV32.

The question to be resolved here is if it really matters. Pedantically, it matters: if the JSON should match the text ISA spec, shamt in RV32 is 5 bits. In reality, though, that 6th bit that would presumably be at position 25 in RV32 representations as it is in the respective RV64 representations, is always zero for these instructions (SLLI, SRLI, SRAI), as can also be seen in the same image above. So, for encoding and decoding, it has no practical impact.

So, practically, it can be ignored. The disadvantage to ignoring, though, is that the JSON cannot be used to generate the content for projects like binutils (or binutils would have to similarly ignore this case).

Hey @ThinkOpenly
can't we set a bit range for shamt and then modify all other related things ?? and then we can transform the JSON as well to use JSON to generate the content for projects.

@ThinkOpenly
Copy link
Owner Author

can't we set a bit range for shamt and then modify all other related things ?? and then we can transform the JSON as well to use JSON to generate the content for projects.

I'll need more details to answer this.

What do you mean by "set a bit range for shamt"? And "modify all other related things"?
And "transform the JSON as well to use JSON..."?

@vishisht-dubey
Copy link

can't we set a bit range for shamt and then modify all other related things ?? and then we can transform the JSON as well to use JSON to generate the content for projects.

I'll need more details to answer this.

What do you mean by "set a bit range for shamt"? And "modify all other related things"? And "transform the JSON as well to use JSON..."?

i think i misunderstood just trying to familiarise myself with sail
can you explain what are shamt SLLI SRLI SRAI and what these do??

@vishisht-dubey
Copy link

can't we set a bit range for shamt and then modify all other related things ?? and then we can transform the JSON as well to use JSON to generate the content for projects.

I'll need more details to answer this.
What do you mean by "set a bit range for shamt"? And "modify all other related things"? And "transform the JSON as well to use JSON..."?

i think i misunderstood just trying to familiarise myself with sail can you explain what are shamt SLLI SRLI SRAI and what these do??

SLLI, SRLI, SRAI are some kind of instructions and shamt is a text ISA specification in RV32 and RV64 am i correct??

@ThinkOpenly
Copy link
Owner Author

SLLI, SRLI, SRAI are some kind of instructions and shamt is a text ISA specification in RV32 and RV64 am i correct??

Did this discussion move to the RVI Slack channel #team-sailing-downstream? It appears so to me.

@vishisht-dubey
Copy link

SLLI, SRLI, SRAI are some kind of instructions and shamt is a text ISA specification in RV32 and RV64 am i correct??

Did this discussion move to the RVI Slack channel #team-sailing-downstream? It appears so to me.

yeah @ThinkOpenly i got the answer

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

No branches or pull requests

2 participants