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