* [PATCH] gas/doc: RISC-V: Fix Type U instruction
@ 2024-05-24 18:45 Javier Mora
2024-05-26 17:44 ` [PATCH v2] " Javier Mora
0 siblings, 1 reply; 6+ messages in thread
From: Javier Mora @ 2024-05-24 18:45 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]
The RISC-V Type U instruction (.insn u) at
https://sourceware.org/binutils/docs/as/RISC_002dV_002dFormats.html
shows an incorrect format for the `simm20` immediate. The bits should
be sequentially arranged (simm20[19:0]) which is how `gas` treats
them, but instead they appear in the Type J format
(simm20[20|10:1|11|19:12]) in the documentation.
Additionally, I changed all occurrences of `opcode6` to `opcode7`
since the opcode has 7 bits, not 6.
(This is my first patch on this project, so my apologies if the format
is not correct!)
As a side note, and speaking of off-by-one errors, I don't quite like
the bit numbering in the instructions: the numbers appear in the ASCII
box diagrams directly under the field boundaries; they should either
appear slightly to the left (indicating that they refer to the first
bit of the next field) or to the right in the case of the leftmost 31;
otherwise it becomes a little confusing (especially since the leftmost
field appears to have one bit less).
Alternatively, you may go full "Python style limits" and
systematically write the index of the bit to the left of the boundary
-- i.e., replace all those 31 with 32 (and the 30 in Type J with 31).
Otherwise the leftmost field is "different".
A third, more explicit but maybe too verbose option is to include both
boundaries in every field.
Thanks in advance.
Keep up the good work!
[-- Attachment #2: 0001-gas-doc-RISC-V-Fix-U-insn-replace-opcode6-7.patch --]
[-- Type: text/x-patch, Size: 4950 bytes --]
From cd25864a6e215323cd68cdfa0fb23313897415da Mon Sep 17 00:00:00 2001
From: Javier Mora <cousteaulecommandant@gmail.com>
Date: Fri, 24 May 2024 20:10:05 +0200
Subject: [PATCH] gas/doc: RISC-V: Fix U insn; replace opcode6->7
The type U RISC-V instruction format in gas/doc/c-riscv.texi shows the
bit arrangement of the simm20 immediate that belongs to the J type;
it should be simm20[19:0] (or simply simm20).
The current behavior of `gas` matches the proposed change.
Additionally, the opcode is called `opcode6` despite of having 7 bits.
Rename it to `opcode7`.
---
gas/doc/c-riscv.texi | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
index 7484a71798a..1b5e834fc69 100644
--- a/gas/doc/c-riscv.texi
+++ b/gas/doc/c-riscv.texi
@@ -435,7 +435,7 @@ instruction formats:
@display
@multitable @columnfractions .15 .40
-@item opcode @tab Unsigned immediate or opcode name for 7-bits opcode.
+@item opcode7 @tab Unsigned immediate or opcode name for 7-bits opcode.
@item opcode2 @tab Unsigned immediate or opcode name for 2-bits opcode.
@item func7 @tab Unsigned immediate for 7-bits function code.
@item func6 @tab Unsigned immediate for 6-bits function code.
@@ -549,62 +549,62 @@ The following table lists the RISC-V instruction formats that are available
with the @samp{.insn} pseudo directive:
@table @code
-@item R type: .insn r opcode6, func3, func7, rd, rs1, rs2
+@item R type: .insn r opcode7, func3, func7, rd, rs1, rs2
@verbatim
+-------+-----+-----+-------+----+---------+
-| func7 | rs2 | rs1 | func3 | rd | opcode6 |
+| func7 | rs2 | rs1 | func3 | rd | opcode7 |
+-------+-----+-----+-------+----+---------+
31 25 20 15 12 7 0
@end verbatim
-@item R type with 4 register operands: .insn r opcode6, func3, func2, rd, rs1, rs2, rs3
-@itemx R4 type: .insn r4 opcode6, func3, func2, rd, rs1, rs2, rs3
+@item R type with 4 register operands: .insn r opcode7, func3, func2, rd, rs1, rs2, rs3
+@itemx R4 type: .insn r4 opcode7, func3, func2, rd, rs1, rs2, rs3
@verbatim
+-----+-------+-----+-----+-------+----+---------+
-| rs3 | func2 | rs2 | rs1 | func3 | rd | opcode6 |
+| rs3 | func2 | rs2 | rs1 | func3 | rd | opcode7 |
+-----+-------+-----+-----+-------+----+---------+
31 27 25 20 15 12 7 0
@end verbatim
-@item I type: .insn i opcode6, func3, rd, rs1, simm12
-@itemx I type: .insn i opcode6, func3, rd, simm12(rs1)
+@item I type: .insn i opcode7, func3, rd, rs1, simm12
+@itemx I type: .insn i opcode7, func3, rd, simm12(rs1)
@verbatim
+--------------+-----+-------+----+---------+
-| simm12[11:0] | rs1 | func3 | rd | opcode6 |
+| simm12[11:0] | rs1 | func3 | rd | opcode7 |
+--------------+-----+-------+----+---------+
31 20 15 12 7 0
@end verbatim
-@item S type: .insn s opcode6, func3, rs2, simm12(rs1)
+@item S type: .insn s opcode7, func3, rs2, simm12(rs1)
@verbatim
+--------------+-----+-----+-------+-------------+---------+
-| simm12[11:5] | rs2 | rs1 | func3 | simm12[4:0] | opcode6 |
+| simm12[11:5] | rs2 | rs1 | func3 | simm12[4:0] | opcode7 |
+--------------+-----+-----+-------+-------------+---------+
31 25 20 15 12 7 0
@end verbatim
-@item B type: .insn s opcode6, func3, rs1, rs2, symbol
-@itemx SB type: .insn sb opcode6, func3, rs1, rs2, symbol
+@item B type: .insn s opcode7, func3, rs1, rs2, symbol
+@itemx SB type: .insn sb opcode7, func3, rs1, rs2, symbol
@verbatim
+-----------------+-----+-----+-------+----------------+---------+
-| simm12[12|10:5] | rs2 | rs1 | func3 | simm12[4:1|11] | opcode6 |
+| simm12[12|10:5] | rs2 | rs1 | func3 | simm12[4:1|11] | opcode7 |
+-----------------+-----+-----+-------+----------------+---------+
31 25 20 15 12 7 0
@end verbatim
-@item U type: .insn u opcode6, rd, simm20
+@item U type: .insn u opcode7, rd, simm20
@verbatim
-+--------------------------+----+---------+
-| simm20[20|10:1|11|19:12] | rd | opcode6 |
-+--------------------------+----+---------+
-31 12 7 0
++--------+----+---------+
+| simm20 | rd | opcode7 |
++--------+----+---------+
+31 12 7 0
@end verbatim
-@item J type: .insn j opcode6, rd, symbol
-@itemx UJ type: .insn uj opcode6, rd, symbol
+@item J type: .insn j opcode7, rd, symbol
+@itemx UJ type: .insn uj opcode7, rd, symbol
@verbatim
+------------+--------------+------------+---------------+----+---------+
-| simm20[20] | simm20[10:1] | simm20[11] | simm20[19:12] | rd | opcode6 |
+| simm20[20] | simm20[10:1] | simm20[11] | simm20[19:12] | rd | opcode7 |
+------------+--------------+------------+---------------+----+---------+
31 30 21 20 12 7 0
@end verbatim
--
2.40.1.423.g2807bd2c10
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] gas/doc: RISC-V: Fix Type U instruction
2024-05-24 18:45 [PATCH] gas/doc: RISC-V: Fix Type U instruction Javier Mora
@ 2024-05-26 17:44 ` Javier Mora
2024-05-28 3:52 ` Nelson Chu
0 siblings, 1 reply; 6+ messages in thread
From: Javier Mora @ 2024-05-26 17:44 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]
On second thought, I just realized that all the simmN/uimmN
instructions use the bit range explicitly, even when it's contiguous
(e.g. `simm12[11:0]`, not just `simm12`). I guess this makes sense
because certain immediates don't take the range [N-1:0] but [N:1], and
one might even think that this .insn would get bits [31:12] of the
immediate value passed in the assembly instruction, when it's actually
bits [19:0] (e.g., `.insn u CUSTOM_0, a0, 0x00042000` will actually
become `0x4200050B`, not `0x0004250B`).
Sending amended patch with the correct `simm20[19:0]`.
El vie, 24 may 2024 a las 20:45, Javier Mora
(<cousteaulecommandant@gmail.com>) escribió:
>
> The RISC-V Type U instruction (.insn u) at
> https://sourceware.org/binutils/docs/as/RISC_002dV_002dFormats.html
> shows an incorrect format for the `simm20` immediate. The bits should
> be sequentially arranged (simm20[19:0]) which is how `gas` treats
> them, but instead they appear in the Type J format
> (simm20[20|10:1|11|19:12]) in the documentation.
> Additionally, I changed all occurrences of `opcode6` to `opcode7`
> since the opcode has 7 bits, not 6.
>
> (This is my first patch on this project, so my apologies if the format
> is not correct!)
>
> As a side note, and speaking of off-by-one errors, I don't quite like
> the bit numbering in the instructions: the numbers appear in the ASCII
> box diagrams directly under the field boundaries; they should either
> appear slightly to the left (indicating that they refer to the first
> bit of the next field) or to the right in the case of the leftmost 31;
> otherwise it becomes a little confusing (especially since the leftmost
> field appears to have one bit less).
> Alternatively, you may go full "Python style limits" and
> systematically write the index of the bit to the left of the boundary
> -- i.e., replace all those 31 with 32 (and the 30 in Type J with 31).
> Otherwise the leftmost field is "different".
> A third, more explicit but maybe too verbose option is to include both
> boundaries in every field.
>
> Thanks in advance.
> Keep up the good work!
[-- Attachment #2: 0001-gas-doc-RISC-V-Fix-U-insn-replace-opcode6-7.patch --]
[-- Type: text/x-patch, Size: 4952 bytes --]
From b4dcdf8cae0290fd98f79144feb90ed10416c45d Mon Sep 17 00:00:00 2001
From: Javier Mora <cousteaulecommandant@gmail.com>
Date: Fri, 24 May 2024 20:10:05 +0200
Subject: [PATCH] gas/doc: RISC-V: Fix U insn; replace opcode6->7
The type U RISC-V instruction format in gas/doc/c-riscv.texi shows the
bit arrangement of the simm20 immediate that belongs to the J type;
it should be just `simm20[19:0]`.
The current behavior of `gas` matches the proposed documentation change.
Additionally, the opcode is called `opcode6` despite of having 7 bits.
Rename it to `opcode7`.
---
gas/doc/c-riscv.texi | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
index 7484a71798a..1b5e834fc69 100644
--- a/gas/doc/c-riscv.texi
+++ b/gas/doc/c-riscv.texi
@@ -435,7 +435,7 @@ instruction formats:
@display
@multitable @columnfractions .15 .40
-@item opcode @tab Unsigned immediate or opcode name for 7-bits opcode.
+@item opcode7 @tab Unsigned immediate or opcode name for 7-bits opcode.
@item opcode2 @tab Unsigned immediate or opcode name for 2-bits opcode.
@item func7 @tab Unsigned immediate for 7-bits function code.
@item func6 @tab Unsigned immediate for 6-bits function code.
@@ -549,62 +549,62 @@ The following table lists the RISC-V instruction formats that are available
with the @samp{.insn} pseudo directive:
@table @code
-@item R type: .insn r opcode6, func3, func7, rd, rs1, rs2
+@item R type: .insn r opcode7, func3, func7, rd, rs1, rs2
@verbatim
+-------+-----+-----+-------+----+---------+
-| func7 | rs2 | rs1 | func3 | rd | opcode6 |
+| func7 | rs2 | rs1 | func3 | rd | opcode7 |
+-------+-----+-----+-------+----+---------+
31 25 20 15 12 7 0
@end verbatim
-@item R type with 4 register operands: .insn r opcode6, func3, func2, rd, rs1, rs2, rs3
-@itemx R4 type: .insn r4 opcode6, func3, func2, rd, rs1, rs2, rs3
+@item R type with 4 register operands: .insn r opcode7, func3, func2, rd, rs1, rs2, rs3
+@itemx R4 type: .insn r4 opcode7, func3, func2, rd, rs1, rs2, rs3
@verbatim
+-----+-------+-----+-----+-------+----+---------+
-| rs3 | func2 | rs2 | rs1 | func3 | rd | opcode6 |
+| rs3 | func2 | rs2 | rs1 | func3 | rd | opcode7 |
+-----+-------+-----+-----+-------+----+---------+
31 27 25 20 15 12 7 0
@end verbatim
-@item I type: .insn i opcode6, func3, rd, rs1, simm12
-@itemx I type: .insn i opcode6, func3, rd, simm12(rs1)
+@item I type: .insn i opcode7, func3, rd, rs1, simm12
+@itemx I type: .insn i opcode7, func3, rd, simm12(rs1)
@verbatim
+--------------+-----+-------+----+---------+
-| simm12[11:0] | rs1 | func3 | rd | opcode6 |
+| simm12[11:0] | rs1 | func3 | rd | opcode7 |
+--------------+-----+-------+----+---------+
31 20 15 12 7 0
@end verbatim
-@item S type: .insn s opcode6, func3, rs2, simm12(rs1)
+@item S type: .insn s opcode7, func3, rs2, simm12(rs1)
@verbatim
+--------------+-----+-----+-------+-------------+---------+
-| simm12[11:5] | rs2 | rs1 | func3 | simm12[4:0] | opcode6 |
+| simm12[11:5] | rs2 | rs1 | func3 | simm12[4:0] | opcode7 |
+--------------+-----+-----+-------+-------------+---------+
31 25 20 15 12 7 0
@end verbatim
-@item B type: .insn s opcode6, func3, rs1, rs2, symbol
-@itemx SB type: .insn sb opcode6, func3, rs1, rs2, symbol
+@item B type: .insn s opcode7, func3, rs1, rs2, symbol
+@itemx SB type: .insn sb opcode7, func3, rs1, rs2, symbol
@verbatim
+-----------------+-----+-----+-------+----------------+---------+
-| simm12[12|10:5] | rs2 | rs1 | func3 | simm12[4:1|11] | opcode6 |
+| simm12[12|10:5] | rs2 | rs1 | func3 | simm12[4:1|11] | opcode7 |
+-----------------+-----+-----+-------+----------------+---------+
31 25 20 15 12 7 0
@end verbatim
-@item U type: .insn u opcode6, rd, simm20
+@item U type: .insn u opcode7, rd, simm20
@verbatim
-+--------------------------+----+---------+
-| simm20[20|10:1|11|19:12] | rd | opcode6 |
-+--------------------------+----+---------+
-31 12 7 0
++--------+----+---------+
+| simm20 | rd | opcode7 |
++--------+----+---------+
+31 12 7 0
@end verbatim
-@item J type: .insn j opcode6, rd, symbol
-@itemx UJ type: .insn uj opcode6, rd, symbol
+@item J type: .insn j opcode7, rd, symbol
+@itemx UJ type: .insn uj opcode7, rd, symbol
@verbatim
+------------+--------------+------------+---------------+----+---------+
-| simm20[20] | simm20[10:1] | simm20[11] | simm20[19:12] | rd | opcode6 |
+| simm20[20] | simm20[10:1] | simm20[11] | simm20[19:12] | rd | opcode7 |
+------------+--------------+------------+---------------+----+---------+
31 30 21 20 12 7 0
@end verbatim
--
2.40.1.423.g2807bd2c10
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] gas/doc: RISC-V: Fix Type U instruction
2024-05-26 17:44 ` [PATCH v2] " Javier Mora
@ 2024-05-28 3:52 ` Nelson Chu
2024-05-30 1:15 ` Nelson Chu
0 siblings, 1 reply; 6+ messages in thread
From: Nelson Chu @ 2024-05-28 3:52 UTC (permalink / raw)
To: Javier Mora; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]
Hi,
This fix looks good and reasonable. Besides, since it looks quite an
institute and a small document fix, I think it should be fine to commit for
now.
If you are interesting in further contributions, then it would be great and
convenient if you have the GNU copyright assignment. Please see the
details as follow,
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright
You can fill one of them (probably, seem like all three look the same ),
and then send it to assign@gnu.org, to get your copyright assignment.
Thanks
Nelson
On Mon, May 27, 2024 at 1:45 AM Javier Mora <cousteaulecommandant@gmail.com>
wrote:
> On second thought, I just realized that all the simmN/uimmN
> instructions use the bit range explicitly, even when it's contiguous
> (e.g. `simm12[11:0]`, not just `simm12`). I guess this makes sense
> because certain immediates don't take the range [N-1:0] but [N:1], and
> one might even think that this .insn would get bits [31:12] of the
> immediate value passed in the assembly instruction, when it's actually
> bits [19:0] (e.g., `.insn u CUSTOM_0, a0, 0x00042000` will actually
> become `0x4200050B`, not `0x0004250B`).
>
> Sending amended patch with the correct `simm20[19:0]`.
The simm20[19:0] is correct if the immediate is a constant. If the
immediate is symbol/label, then with the use of
%hi/%pcrel_hi/%got_pcrel_hi/... will give you simm[31:12] of the symbol,
but that will also generate the corresponding relocation
R_RISCV_HI20/PCREL_HI20/GOT_PCREL_HI20/... for the instruction.
> > As a side note, and speaking of off-by-one errors, I don't quite like
> > the bit numbering in the instructions: the numbers appear in the ASCII
> > box diagrams directly under the field boundaries; they should either
> > appear slightly to the left (indicating that they refer to the first
> > bit of the next field) or to the right in the case of the leftmost 31;
> > otherwise it becomes a little confusing (especially since the leftmost
> > field appears to have one bit less).
> > Alternatively, you may go full "Python style limits" and
> > systematically write the index of the bit to the left of the boundary
> > -- i.e., replace all those 31 with 32 (and the 30 in Type J with 31).
> > Otherwise the leftmost field is "different".
> > A third, more explicit but maybe too verbose option is to include both
> > boundaries in every field.
>
I can just apply the patch, not sure what happened.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] gas/doc: RISC-V: Fix Type U instruction
2024-05-28 3:52 ` Nelson Chu
@ 2024-05-30 1:15 ` Nelson Chu
2024-05-30 15:35 ` Javier Mora
0 siblings, 1 reply; 6+ messages in thread
From: Nelson Chu @ 2024-05-30 1:15 UTC (permalink / raw)
To: Javier Mora; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 839 bytes --]
On Tue, May 28, 2024 at 11:52 AM Nelson Chu <nelson@rivosinc.com> wrote:
> Hi,
>
> This fix looks good and reasonable. Besides, since it looks quite an
> institute and a small document fix, I think it should be fine to commit for
> now.
>
> If you are interesting in further contributions, then it would be great
> and convenient if you have the GNU copyright assignment. Please see the
> details as follow,
>
> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright
>
> You can fill one of them (probably, seem like all three look the same ),
> and then send it to assign@gnu.org, to get your copyright assignment.
>
Or you can add the "Signed-off-by: your name <your email>" in the commit
message every time, that also work.
https://sourceware.org/pipermail/binutils/2022-October/123680.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] gas/doc: RISC-V: Fix Type U instruction
2024-05-30 1:15 ` Nelson Chu
@ 2024-05-30 15:35 ` Javier Mora
2024-05-31 8:10 ` Nelson Chu
0 siblings, 1 reply; 6+ messages in thread
From: Javier Mora @ 2024-05-30 15:35 UTC (permalink / raw)
To: Nelson Chu; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]
Do I need to do that now (for this patch), or only for future patches?
El jue, 30 may 2024, 3:15, Nelson Chu <nelson@rivosinc.com> escribió:
>
>
> On Tue, May 28, 2024 at 11:52 AM Nelson Chu <nelson@rivosinc.com> wrote:
>
>> Hi,
>>
>> This fix looks good and reasonable. Besides, since it looks quite an
>> institute and a small document fix, I think it should be fine to commit for
>> now.
>>
>> If you are interesting in further contributions, then it would be great
>> and convenient if you have the GNU copyright assignment. Please see the
>> details as follow,
>>
>> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright
>>
>> You can fill one of them (probably, seem like all three look the same ),
>> and then send it to assign@gnu.org, to get your copyright assignment.
>>
>
> Or you can add the "Signed-off-by: your name <your email>" in the commit
> message every time, that also work.
> https://sourceware.org/pipermail/binutils/2022-October/123680.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] gas/doc: RISC-V: Fix Type U instruction
2024-05-30 15:35 ` Javier Mora
@ 2024-05-31 8:10 ` Nelson Chu
0 siblings, 0 replies; 6+ messages in thread
From: Nelson Chu @ 2024-05-31 8:10 UTC (permalink / raw)
To: Javier Mora; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]
Future patches :-)
Nelson
On Thu, May 30, 2024 at 11:35 PM Javier Mora <cousteaulecommandant@gmail.com>
wrote:
> Do I need to do that now (for this patch), or only for future patches?
>
> El jue, 30 may 2024, 3:15, Nelson Chu <nelson@rivosinc.com> escribió:
>
>>
>>
>> On Tue, May 28, 2024 at 11:52 AM Nelson Chu <nelson@rivosinc.com> wrote:
>>
>>> Hi,
>>>
>>> This fix looks good and reasonable. Besides, since it looks quite an
>>> institute and a small document fix, I think it should be fine to commit for
>>> now.
>>>
>>> If you are interesting in further contributions, then it would be great
>>> and convenient if you have the GNU copyright assignment. Please see the
>>> details as follow,
>>>
>>> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright
>>>
>>> You can fill one of them (probably, seem like all three look the same ),
>>> and then send it to assign@gnu.org, to get your copyright assignment.
>>>
>>
>> Or you can add the "Signed-off-by: your name <your email>" in the commit
>> message every time, that also work.
>> https://sourceware.org/pipermail/binutils/2022-October/123680.html
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-31 8:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-24 18:45 [PATCH] gas/doc: RISC-V: Fix Type U instruction Javier Mora
2024-05-26 17:44 ` [PATCH v2] " Javier Mora
2024-05-28 3:52 ` Nelson Chu
2024-05-30 1:15 ` Nelson Chu
2024-05-30 15:35 ` Javier Mora
2024-05-31 8:10 ` Nelson Chu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).