[1/3] RISC-V: Fix doc for .insn

Message ID 1558939175-51429-1-git-send-email-kito@andestech.com
State New
Headers show
Series
  • [1/3] RISC-V: Fix doc for .insn
Related show

Commit Message

Kito Cheng May 27, 2019, 6:39 a.m.
From: Kito Cheng <kito.cheng@gmail.com>


gas/ChangeLog:

	* doc/c-riscv.texi (Instruction Formats): Fix encoding table for SB
	type and fix typo.
---
 gas/doc/c-riscv.texi | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
1.8.3.1

Comments

Jim Wilson May 29, 2019, 6:12 a.m. | #1
On Sun, May 26, 2019 at 11:37 PM Kito Cheng <kito@andestech.com> wrote:
>         * doc/c-riscv.texi (Instruction Formats): Fix encoding table for SB

>         type and fix typo.


This is OK.

Jim
Jim Wilson May 29, 2019, 6:40 a.m. | #2
On Sun, May 26, 2019 at 11:37 PM Kito Cheng <kito@andestech.com> wrote:
> @@ -344,10 +344,10 @@ with the @samp{.insn} pseudo directive:

>  @item SB type: .insn sb opcode, func3, rd, rs1, symbol

>  @itemx SB type: .insn sb opcode, func3, rd, simm12(rs1)

>  @verbatim

> -+--------------+-----+-----+-------+-------------+-------------+

> -| simm21[11:5] | rs2 | rs1 | func3 | simm12[4:0] |      opcode |

> -+--------------+-----+-----+-------+-------------+-------------+

> -31             25    20    15      12            7             0

> ++------------+--------------+-----+-----+-------+-------------+-------------+--------+

> +| simm12[12] | simm12[10:5] | rs2 | rs1 | func3 | simm12[4:1] | simm12[11]] | opcode |

> ++------------+--------------+-----+-----+-------+-------------+-------------+--------+

> +31          30            25    20    15      12           7            0


Actually, on second thought, this looks a little confused.  The
testcase gas/testsuite/gas/riscv/insn.s uses sb for both stores and
branches.  If sb is a store, then the old description is correct.  if
sb is a branch, then the new description is correct.  But stores never
should have been using sb, they should be using s.  And the testcase
is using s for loads which is wrong, which prevents us from using s
for stores.  This looks like a bug in the original implementation
which we may not be able to fix without breaking backward
compatibility.  The .insn pseudo op is not used much, and probably not
used for loads and stores, so maybe it is OK to break this to get the
implementation correct.

Otherwise, the doc change looks correct, it just exposed a bug in the
implementation and the testcase.

Jim

Patch

diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
index 42d1ce3..5c30db1 100644
--- a/gas/doc/c-riscv.texi
+++ b/gas/doc/c-riscv.texi
@@ -286,7 +286,7 @@  Opcode space for msub instruction.
 @item AMO
 Opcode space for atomic memory operation instructions.
 
-@item MISC_IMM
+@item MISC_MEM
 Opcode space for misc instructions.
 
 @item SYSTEM
@@ -344,10 +344,10 @@  with the @samp{.insn} pseudo directive:
 @item SB type: .insn sb opcode, func3, rd, rs1, symbol
 @itemx SB type: .insn sb opcode, func3, rd, simm12(rs1)
 @verbatim
-+--------------+-----+-----+-------+-------------+-------------+
-| simm21[11:5] | rs2 | rs1 | func3 | simm12[4:0] |      opcode |
-+--------------+-----+-----+-------+-------------+-------------+
-31             25    20    15      12            7             0
++------------+--------------+-----+-----+-------+-------------+-------------+--------+
+| simm12[12] | simm12[10:5] | rs2 | rs1 | func3 | simm12[4:1] | simm12[11]] | opcode |
++------------+--------------+-----+-----+-------+-------------+-------------+--------+
+31          30            25    20    15      12           7            0
 @end verbatim
 
 @item U type: .insn u opcode, rd, simm20