<div dir="ltr">I will vote for option 1, it looks better :-)<div><br></div><div>Thanks</div><div>Nelson</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 18, 2024 at 10:41 PM Javier Mora <<a href="mailto:cousteaulecommandant@gmail.com">cousteaulecommandant@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I found an issue with the bit numbering used in the RISC-V instruction<br>
formats shown at<br>
<<a href="https://sourceware.org/binutils/docs/as/RISC_002dV_002dFormats.html" rel="noreferrer" target="_blank">https://sourceware.org/binutils/docs/as/RISC_002dV_002dFormats.html</a>>:<br>
<br>
```<br>
R type: .insn r opcode7, func3, func7, rd, rs1, rs2<br>
<br>
+-------+-----+-----+-------+----+---------+<br>
| func7 | rs2 | rs1 | func3 | rd | opcode7 |<br>
+-------+-----+-----+-------+----+---------+<br>
31 25 20 15 12 7 0<br>
<br>
J type: .insn j opcode7, rd, symbol<br>
UJ type: .insn uj opcode7, rd, symbol<br>
<br>
+----------+------------+----------+-------------+----+---------+<br>
| simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |<br>
+----------+------------+----------+-------------+----+---------+<br>
31 30 21 20 12 7 0<br>
```<br>
<br>
The bit indexes written down below the bit layout are confusing and<br>
misleading, since it is not clear if they refer to the bit in which a<br>
field starts or the one in which it ends.<br>
In other words, one can interpret that opcode7's "7...0" means that<br>
the field goes from bit 0 (included) to bit 7 (not included), i.e.,<br>
bits 0 to 6, so 7 bits total; but then it says func7 uses bits<br>
"31...25", which following the same reasoning would be 6 bits, not 7;<br>
and the whole instruction would use bits "31...0" which would be 31<br>
bits instead of 32.<br>
(TL;DR there's an off-by-one error.)<br>
<br>
Overall, it seems that all the instructions just state the rightmost<br>
bit for all fields, and then add the leftmost bit of the leftmost<br>
field (31, or 15 for 16-bit instructions), which is confusing since<br>
all the numbers are written exactly below the `|` separating fields.<br>
And then there's the particularly confusing case of J/UJ instruction<br>
whose leftmost field is 1 bit wide, and it indicates "31" and "30".<br>
<br>
The indexes should be changed to become clearer and better reflect the<br>
field boundaries.<br>
I have thought of several alternatives but wanted to ask your opinion<br>
before submitting a patch.<br>
<br>
Option 1: make the number always refer to the bit to the left of the<br>
`|`, and 32 for the leftmost bit (so e.g. "32...25" means that the<br>
field spans the bit range `[25, 32)`):<br>
<br>
```<br>
+-------+-----+-----+-------+----+---------+<br>
| func7 | rs2 | rs1 | func3 | rd | opcode7 |<br>
+-------+-----+-----+-------+----+---------+<br>
32 25 20 15 12 7 0<br>
<br>
+----------+------------+----------+-------------+----+---------+<br>
| simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |<br>
+----------+------------+----------+-------------+----+---------+<br>
32 31 21 20 12 7 0<br>
```<br>
<br>
Option 2: keep the numbers as they are now, but shift them left or<br>
right a little bit so that they're to the left or the right of the `|`<br>
and it becomes clear which bit they refer to:<br>
<br>
```<br>
+-------+-----+-----+-------+----+---------+<br>
| func7 | rs2 | rs1 | func3 | rd | opcode7 |<br>
+-------+-----+-----+-------+----+---------+<br>
31 25 20 15 12 7 0<br>
<br>
+----------+------------+----------+-------------+----+---------+<br>
| simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |<br>
+----------+------------+----------+-------------+----+---------+<br>
31 21 20 12 7 0<br>
```<br>
<br>
Option 3: include first and last index of all fields (too verbose, but<br>
the most explicit option):<br>
<br>
```<br>
+-------+-------+-------+-------+------+---------+<br>
| func7 | rs2 | rs1 | func3 | rd | opcode7 |<br>
+-------+-------+-------+-------+------+---------+<br>
31 25 24 20 19 15 14 12 11 7 6 0<br>
<br>
+----------+------------+----------+-------------+------+---------+<br>
| simm[20] | simm[10:1] | simm[11] | simm[19:12] | rd | opcode7 |<br>
+----------+------------+----------+-------------+------+---------+<br>
31 30 21 20 19 12 11 7 6 0<br>
```<br>
<br>
Personally I'd go for Option 2. What do you think?<br>
<br>
I can submit a patch if needed.<br>
<br>
Regards,<br>
Javier Mora<br>
</blockquote></div>