Wren6991/Hazard3

BEXTM(I) Encoding Tables are Incorrect

lenary opened this issue · 3 comments

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).

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.

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

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).