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

BEXTM(I) Encoding Tables are Incorrect #17

Closed
lenary opened this issue Aug 29, 2024 · 3 comments
Closed

BEXTM(I) Encoding Tables are Incorrect #17

lenary opened this issue Aug 29, 2024 · 3 comments

Comments

@lenary
Copy link

lenary commented Aug 29, 2024

The following two custom instruction encoding tables are slightly wrong:

| Bits | Name | Value | Description
| 31:29 | `funct7[6:4]` | `0b000` | RES0
| 28:26 | `size` | - | Number of ones in mask, values 0->7 encode 1->8 bits.
| 25 | `funct7[0]` | `0b0` | RES0, because aligns with `shamt[5]` of potential RV64 version of `h3.bextmi`
| 24:20 | `rs2` | - | Source register 2 (shift amount)
| 19:15 | `rs1` | - | Source register 1
| 14:12 | `funct3` | `0b000` | `h3.bextm`
| 11:7 | `rd` | - | Destination register
| 6:2 | `opc` | `0b01011`| custom0 opcode
| 1:0 | `size` | `0b11` | 32-bit instruction

| Bits | Name | Value | Description
| 31:29 | `imm[11:9]` | `0b000` | RES0
| 28:26 | `size` | - | Number of ones in mask, values 0->7 encode 1->8 bits.
| 25 | `imm[5]` | `0b0` | RES0, for potential future RV64 version
| 24:20 | `shamt` | - | Shift amount, 0 through 31
| 19:15 | `rs1` | - | Source register 1
| 14:12 | `funct3` | `0b100` | `h3.bextmi`
| 11:7 | `rd` | - | Destination register
| 6:2 | `opc` | `0b01011`| custom0 opcode
| 1:0 | `size` | `0b11` | 32-bit instruction

The issue is with bits 6:0 (the final two rows): these tables claim that:

  • bits 1:0 should be 11
  • bits 6:2 should be 01011

When using the custom-0 opcode, according to Table 70 of the unpriviliged spec ("RISC-V base opcode map"), the following instruction encoding is described:

  • bits 1:0 should be 11 (from the table header)
  • bits 4:2 should be 010 (from header row)
  • bits 6:5 should be 00 (from first column)

It looks like the problem here is that when you separated out the size field, you ended up repeating the bits from the opcode -- which implicitly encode the instruction size -- into your opc field.

The assembler directives using .insn I, 0x0b ... work correctly, because these directives put the 0xb value into bits 6:0, rather than knowing that an I-type instruction is always 32-bits and putting the 0xb value into bits 6:2 (and 0b11 into bits 1:0).

@Wren6991
Copy link
Owner

Good spot, thank you. For avoidance of doubt the encoding used by hardware is this one:

Hazard3/hdl/rv_opcodes.vh

Lines 151 to 153 in a4412c0

// Xh3b (Hazard3 custom bitmanip): currently just a multi-bit version of bext/bexti from Zbs
`define RVOPC_H3_BEXTM 32'b000???0??????????000?????0001011 // custom-0 funct3=0
`define RVOPC_H3_BEXTMI 32'b000???0??????????100?????0001011 // custom-0 funct3=4

So as you say, the mistake in the table is it documents bits 1:0 twice. I'll fix the table.

@Wren6991
Copy link
Owner

(also the comment in that header is outdated; the extension is Xh3bextm and it consists solely of these two instructions)

Wren6991 added a commit that referenced this issue Aug 29, 2024
…le instruction macros.

Also fix outdated comment about these encodings in rv_opcodes.vh
@Wren6991
Copy link
Owner

Wren6991 commented Aug 29, 2024

I added a note on this to the v1.0 release notes. I'm not going to update the PDF as there are other docs changes ahead of this on the develop branch, and I don't want to add a dirty PDF build to the v1.0 release.

I will release a new PDF with the v1.1 release, which will contain this fix. It will document the same versions of all existing custom extensions as the v1.0 PDF (which for the record I consider to be extension versions 1.0).

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