Re: [PATCH v2 3/3] RISC-V: PR27916, Extend .insn directive to support hardcode encoding.

[Kito Cheng <kito.cheng@sifive.com>]

This patch is extending new formats of .insn, so I suggest this should
be a standalone patch.

On Fri, Jul 9, 2021 at 3:28 PM Nelson Chu <nelson.chu@sifive.com> wrote:
>
> The .insn directive can let users use their own instructions, or
> some new instruction, which haven't supported in the old binutils.
> For example, if users want to use sifive cache instruction, they
> cannot just write "cflush.d1.l1" in the assembly code, they should
> use ".insn i SYSTEM, 0, x0, x10, -0x40".  But the .insn directive
> may not easy to use for some cases, and not so friendly to users.
> Therefore, I believe most of the users will use ".word 0xfc050073",
> to encode the instructions directly, rather than use .insn.  But
> once we have supported the mapping symbols, the .word directives
> are marked as data, so disassembler won't dump them as instructions
> as usual.  I have discussed this with Kito many times, we all think
> extend the .insn direcitve to support the hardcode encoding, is the
> easiest way to resolve the problem.  Therefore, there are two more
> .insn formats are proposed as follows,
>
> (original) .insn <type>, <operand1>, <operand2>, ...
>            .insn <insn-length>, <value>
>            .insn <value>
>
> The <type> is string, and the <insn-length> and <value> are constants.
>
> ChangeLog:
>
> gas/
>
>         pr 27916
>         * config/tc-riscv.c (riscv_ip_hardcode): Similar to riscv_ip,
>         but assembles an instruction according to the hardcode values
>         of .insn directive.
>         * testsuite/gas/riscv/insn-fail.d: New testcases.
>         * testsuite/gas/riscv/insn-fail.l: Likewise.
>         * testsuite/gas/riscv/insn-fail.s: Likewise.
>         * testsuite/gas/riscv/insn.d: Updated.
>         * testsuite/gas/riscv/insn.s: Likewise.
> ---
>  gas/config/tc-riscv.c               | 57 +++++++++++++++++++++++++++--
>  gas/testsuite/gas/riscv/insn-fail.d |  3 ++
>  gas/testsuite/gas/riscv/insn-fail.l |  7 ++++
>  gas/testsuite/gas/riscv/insn-fail.s |  6 +++
>  gas/testsuite/gas/riscv/insn.d      |  6 +++
>  gas/testsuite/gas/riscv/insn.s      |  7 ++++
>  6 files changed, 83 insertions(+), 3 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/insn-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/insn-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/insn-fail.s
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 55c49471825..e9be17f56d1 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -2922,6 +2922,50 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
>    return error;
>  }
>
> +/* Similar to riscv_ip, but assembles an instruction according to the
> +   hardcode values of .insn directive.  */
> +
> +static const char *
> +riscv_ip_hardcode (char *str,
> +                  struct riscv_cl_insn *ip,
> +                  expressionS *imm_expr,
> +                  const char *error)
> +{
> +  struct riscv_opcode *insn;
> +  insn_t values[2] = {0, 0};
> +  unsigned int num = 0;
> +
> +  input_line_pointer = str;
> +  do
> +    {
> +      expression (imm_expr);
> +      if (imm_expr->X_op != O_constant)
> +       {
> +         /* The first value isn't constant, so it should be
> +            .insn <type> <operands>.  Call riscv_ip to parse it.  */
> +         if (num == 0)
> +           return error;
> +         return _("values must be constant");
> +       }
> +      values[num++] = (insn_t) imm_expr->X_add_number;
> +    }
> +  while (*input_line_pointer++ == ',' && num < 2);
> +
> +  input_line_pointer--;
> +  if (*input_line_pointer != '\0')
> +    return _("unrecognized values");
> +
> +  insn = XNEW (struct riscv_opcode);
> +  insn->match = values[num - 1];
> +  create_insn (ip, insn);
> +  unsigned int bytes = riscv_insn_length (insn->match);
> +  if (values[num - 1] >> (8 * bytes) != 0
> +      || (num == 2 && values[0] != bytes))
> +    return _("value conflicts with instruction length");
> +
> +  return NULL;
> +}
> +
>  void
>  md_assemble (char *str)
>  {
> @@ -3914,7 +3958,10 @@ s_riscv_leb128 (int sign)
>    return s_leb128 (sign);
>  }
>
> -/* Parse the .insn directive.  */
> +/* Parse the .insn directive.  There are three formats,
> +   Format 1: .insn <type> <operand1>, <operand2>, ...
> +   Format 2: .insn <length>, <value>
> +   Format 3: .insn <value>.  */
>
>  static void
>  s_riscv_insn (int x ATTRIBUTE_UNUSED)
> @@ -3935,11 +3982,15 @@ s_riscv_insn (int x ATTRIBUTE_UNUSED)
>
>    const char *error = riscv_ip (str, &insn, &imm_expr,
>                                 &imm_reloc, insn_type_hash);
> -
>    if (error)
>      {
> -      as_bad ("%s `%s'", error, str);
> +      char *save_in = input_line_pointer;
> +      error = riscv_ip_hardcode (str, &insn, &imm_expr, error);
> +      input_line_pointer = save_in;
>      }
> +
> +  if (error)
> +    as_bad ("%s `%s'", error, str);
>    else
>      {
>        gas_assert (insn.insn_mo->pinfo != INSN_MACRO);
> diff --git a/gas/testsuite/gas/riscv/insn-fail.d b/gas/testsuite/gas/riscv/insn-fail.d
> new file mode 100644
> index 00000000000..3548e85415a
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/insn-fail.d
> @@ -0,0 +1,3 @@
> +#as:
> +#source: insn-fail.s
> +#error_output: insn-fail.l
> diff --git a/gas/testsuite/gas/riscv/insn-fail.l b/gas/testsuite/gas/riscv/insn-fail.l
> new file mode 100644
> index 00000000000..e47d106b39b
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/insn-fail.l
> @@ -0,0 +1,7 @@
> +.*Assembler messages:
> +.*Error: unrecognized opcode `r,0x00000013'
> +.*Error: values must be constant `0x4,rs1'
> +.*Error: unrecognized values `0x4 0x5'
> +.*Error: unrecognized values `0x4,0x5,0x6'
> +.*Error: value conflicts with instruction length `0x4,0x0001'
> +.*Error: value conflicts with instruction length `0x2,0x00000013'
> diff --git a/gas/testsuite/gas/riscv/insn-fail.s b/gas/testsuite/gas/riscv/insn-fail.s
> new file mode 100644
> index 00000000000..064211d985d
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/insn-fail.s
> @@ -0,0 +1,6 @@
> +       .insn   r, 0x00000013
> +       .insn   0x4, rs1
> +       .insn   0x4 0x5
> +       .insn   0x4, 0x5, 0x6
> +       .insn   0x4, 0x0001
> +       .insn   0x2, 0x00000013
> diff --git a/gas/testsuite/gas/riscv/insn.d b/gas/testsuite/gas/riscv/insn.d
> index 8cb3d64b1a5..4edacc63368 100644
> --- a/gas/testsuite/gas/riscv/insn.d
> +++ b/gas/testsuite/gas/riscv/insn.d
> @@ -69,3 +69,9 @@ Disassembly of section .text:
>  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
>  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
>  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
> +[^:]+:[        ]+0001[         ]+nop
> +[^:]+:[        ]+00000013[     ]+nop
> +[^:]+:[        ]+00000057[     ]+0x57
> +[^:]+:[        ]+0001[         ]+nop
> +[^:]+:[        ]+00000013[     ]+nop
> +[^:]+:[        ]+00000057[     ]+0x57
> diff --git a/gas/testsuite/gas/riscv/insn.s b/gas/testsuite/gas/riscv/insn.s
> index 937ad119ff2..84739056b1a 100644
> --- a/gas/testsuite/gas/riscv/insn.s
> +++ b/gas/testsuite/gas/riscv/insn.s
> @@ -53,3 +53,10 @@ target:
>         .insn r  0x33,  0,  0, fa0, a1, fa2
>         .insn r  0x33,  0,  0, a0, fa1, fa2
>         .insn r  0x33,  0,  0, fa0, fa1, fa2
> +
> +       .insn 0x0001
> +       .insn 0x00000013
> +       .insn 0x00000057
> +       .insn 0x2, 0x0001
> +       .insn 0x4, 0x00000013
> +       .insn 0x4, 0x00000057
> --
> 2.30.2
>