ThinkOpenly / sail

Sail architecture definition language
Other
10 stars 11 forks source link

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

Open ThinkOpenly opened 5 months ago

ThinkOpenly commented 5 months ago

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 commented 5 months ago

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

ThinkOpenly commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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