[PATCH] RISC-V: Add .insn support

[Jim Wilson]

On Wed, Mar 7, 2018 at 1:15 AM, Kito Cheng <kito.cheng@gmail.com> wrote:
> This patch make  RISC-V assembler support new directive: .insn, it
> able to write instruction with another form just like s/390's .insn
> directive.

I have a bunch of comments, but they are all minor cleanups.  I think
the overall structure of this is fine.

The patch created with "git format-patch" can be mailed with "git send-email".
You can edit the patch after creating it and before emailing, to add info about
the patch, such as what it fixes and how it was tested.

The tc-riscv.c ChangeLog entry is formatted wrong.  Some of the lines have two
extra spaces after the tab.

GNU coding standards says that every declaration should have an explanatory
comment before it.  Most of the new types/variables/functions added to
tc-riscv.c are missing comments.

In opcode_name_list, NMADD and NMSUB are swapped.  JAR should be JAL, as
Andrew mentioned.

"value must be 0 ~ 7"
is inconsistent with the other messages that use ... for a range

The first
"bad FUNCT field specifier 'F%c'\n"
should be CF%c for the compressed FUNCT field specifier, and maybe change
the message also to mention CFUNCT or compressed FUNCT or something

The line handling in s_riscv_insn doesn't look right.  It has an explicit
check for \n.  It should be using is_end_of_line similar to s_riscv_option.
Also, ignoring the rest of the characters on the line after parsing a .insn
is wrong.  We should give an error is there are extra non-comment characters
on the line.  This is done by calling demand_empty_rest_of_line, similar to
s_riscv_option.

In c-riscv.texi
"instructions formats" should be "instruction formats"

has another two references to JAR/jar that should be JAL/jal

In riscv-opc.c
+{"ci",    "C",  "O2,CF3,d,Cj",        0,    0,  match_opcode, 0 },
+{"ci",    "C",  "O2,CF3,d,Co",        0,    0,  match_opcode, 0 },

Co is all rvc immediate constants, Cj is all rvc immediates except 0.  So there
doesn't seem to be any point to having both.  You only need Co.  I did change
this encoding stuff at one point when working on hint instructions, so maybe
this is just left over from the old support.

The .ciw and .ci patterns are the same in the riscv_insn_types array, but are
documented as being different in the c-riscv.texi file.  In the testcase, you
used addi and li as examples for .ci and .ciw, but they use the same format.
There are two formats, using 3-bit register fields, that are missing.  One takes
one register, one takes 2 registers.  Maybe one of these is what you meant
.ciw to be for?  Or maybe the issue is the rd restrictions on addi/li,
but the .insn
support isn't trying to handle them, and I don't think it is
reasonable to expect
it to, since these restrictions are due to overlapping opcodes, not actual
instruction format limitations, so I don't think we should worry about that.  We
should just treat addi and li as the same .insn format, and add the missing two
formats.

Jim