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

Message ID 20210709072825.13709-4-nelson.chu@sifive.com
State New
Headers show
Series
  • RISC-V: The series to supporting mapping symbols
Related show

Commit Message

Nelson Chu July 9, 2021, 7:28 a.m.
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

-- 
2.30.2

Comments

Kito Cheng July 9, 2021, 8:01 a.m. | #1
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

>

Patch

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