gas/doc: RISC-V: Confusing bitfield limits

[Nelson Chu]

I will vote for option 1, it looks better :-)

Thanks
Nelson

On Wed, Sep 18, 2024 at 10:41 PM Javier Mora <cousteaulecommandant@gmail.com>
wrote:

> I found an issue with the bit numbering used in the RISC-V instruction
> formats shown at
> <https://sourceware.org/binutils/docs/as/RISC_002dV_002dFormats.html>:
>
> ```
> R type: .insn r opcode7, func3, func7, rd, rs1, rs2
>
>     +-------+-----+-----+-------+----+---------+
>     | func7 | rs2 | rs1 | func3 | rd | opcode7 |
>     +-------+-----+-----+-------+----+---------+
>     31      25    20    15      12   7        0
>
> J type: .insn j opcode7, rd, symbol
> UJ type: .insn uj opcode7, rd, symbol
>
>     +----------+------------+----------+-------------+----+---------+
>     | simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |
>     +----------+------------+----------+-------------+----+---------+
>     31         30           21         20            12   7         0
> ```
>
> The bit indexes written down below the bit layout are confusing and
> misleading, since it is not clear if they refer to the bit in which a
> field starts or the one in which it ends.
> In other words, one can interpret that opcode7's "7...0" means that
> the field goes from bit 0 (included) to bit 7 (not included), i.e.,
> bits 0 to 6, so 7 bits total; but then it says func7 uses bits
> "31...25", which following the same reasoning would be 6 bits, not 7;
> and the whole instruction would use bits "31...0" which would be 31
> bits instead of 32.
> (TL;DR there's an off-by-one error.)
>
> Overall, it seems that all the instructions just state the rightmost
> bit for all fields, and then add the leftmost bit of the leftmost
> field (31, or 15 for 16-bit instructions), which is confusing since
> all the numbers are written exactly below the `|` separating fields.
> And then there's the particularly confusing case of J/UJ instruction
> whose leftmost field is 1 bit wide, and it indicates "31" and "30".
>
> The indexes should be changed to become clearer and better reflect the
> field boundaries.
> I have thought of several alternatives but wanted to ask your opinion
> before submitting a patch.
>
> Option 1: make the number always refer to the bit to the left of the
> `|`, and 32 for the leftmost bit (so e.g. "32...25" means that the
> field spans the bit range `[25, 32)`):
>
> ```
>     +-------+-----+-----+-------+----+---------+
>     | func7 | rs2 | rs1 | func3 | rd | opcode7 |
>     +-------+-----+-----+-------+----+---------+
>     32     25    20    15      12    7         0
>
>     +----------+------------+----------+-------------+----+---------+
>     | simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |
>     +----------+------------+----------+-------------+----+---------+
>     32        31           21         20            12    7         0
> ```
>
> Option 2: keep the numbers as they are now, but shift them left or
> right a little bit so that they're to the left or the right of the `|`
> and it becomes clear which bit they refer to:
>
> ```
>     +-------+-----+-----+-------+----+---------+
>     | func7 | rs2 | rs1 | func3 | rd | opcode7 |
>     +-------+-----+-----+-------+----+---------+
>      31   25    20    15      12    7         0
>
>     +----------+------------+----------+-------------+----+---------+
>     | simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |
>     +----------+------------+----------+-------------+----+---------+
>              31           21         20            12    7         0
> ```
>
> Option 3: include first and last index of all fields (too verbose, but
> the most explicit option):
>
> ```
>     +-------+-------+-------+-------+------+---------+
>     | func7 |  rs2  |  rs1  | func3 |  rd  | opcode7 |
>     +-------+-------+-------+-------+------+---------+
>      31   25 24   20 19   15 14   12 11   7 6       0
>
>     +----------+------------+----------+-------------+------+---------+
>     | simm[20] | simm[10:1] | simm[11] | simm[19:12] |  rd  | opcode7 |
>     +----------+------------+----------+-------------+------+---------+
>          31     30        21     20     19         12 11   7 6       0
> ```
>
> Personally I'd go for Option 2.  What do you think?
>
> I can submit a patch if needed.
>
> Regards,
>         Javier Mora
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240919/0df66973/attachment.htm>