public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [committed v3] RISC-V: Support new .option arch directive.
@ 2021-11-19 10:53 Nelson Chu
  2021-11-19 16:01 ` Palmer Dabbelt
  0 siblings, 1 reply; 4+ messages in thread
From: Nelson Chu @ 2021-11-19 10:53 UTC (permalink / raw)
  To: binutils, jim.wilson.gcc, kito.cheng, palmer

https://github.com/riscv/riscv-asm-manual/pull/67

Format:
.option arch, +<extension><version>, ...
.option arch, -<extension>
.option arch, =<ISA string>

The new direcitve is used to enable/disable extensions for the specific
code region.  For example,

.attribute arch, "rv64ic"   # arch = rv64i2p0_c2p0
.option push
.option arch, +d2p0, -c     # arch = rv64i2p0_f2p0_d2p0, f is added implied
.option arch, =rv32gc       # arch = rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0
.option pop                 # arch = rv64i2p0_c2p0

Note that,
1. ".option rvc/norvc" have the same behavior as ".option arch +c/-c".
2. ".option arch -i" is illegal, since we cannot remove base i extension.
3. If arch=rv64i2p0, then ".option arch, +i3p0" will update the i's version
   from 2.0 to 3.0.
4. If arch=rv64i3p0, then ".option arch, +i" will update the i's version
   from 2.0 to the default one according to the chosen isa spec.

bfd/
	* elfxx-riscv.c (riscv_add_subset): If the subset is already added,
	and the new versions are not RISCV_UNKNOWN_VERSION, then update the
	versions to the subset list.
	(riscv_copy_subset): New function.  Copy the subset from list.
	(riscv_copy_subset_list): New function.  Return the new copyed list.
	(riscv_update_subset): Updated to make .option arch directives workable.
	* elfxx-riscv.h: Updated.
gas/
	* config/tc-riscv.c (riscv_subsets): Defined as a pointer.
	(riscv_rps_as): Init the subset_list to NULL, we will set it later
	once riscv_opts_stack is created or updated.
	(struct riscv_option_stack, riscv_opts_stack): Moved forward.
	(riscv_set_arch): Updated.
	(s_riscv_option): Support new .option arch directive, to add, remove
	or update subsets for the specific code region.
	(riscv_write_out_attrs): Updated.
	* doc/c-riscv.texi: Added document for new .option arch directive.
	* testsuite/gas/riscv/option-arch-01a.d: New testcase.
	* testsuite/gas/riscv/option-arch-01b.d: Likewise.
	* testsuite/gas/riscv/option-arch-01.s: Likewise..
	* testsuite/gas/riscv/option-arch-02.d: Likewise.
	* testsuite/gas/riscv/option-arch-02.s: Likewise.
	* testsuite/gas/riscv/option-arch-fail.d: Likewise.
	* testsuite/gas/riscv/option-arch-fail.l: Likewise.
	* testsuite/gas/riscv/option-arch-fail.s: Likewise.
---
 bfd/elfxx-riscv.c                          | 158 ++++++++++++++++++++++++-----
 bfd/elfxx-riscv.h                          |   5 +-
 gas/config/tc-riscv.c                      |  57 +++++++----
 gas/doc/c-riscv.texi                       |  15 ++-
 gas/testsuite/gas/riscv/option-arch-01.s   |  10 ++
 gas/testsuite/gas/riscv/option-arch-01a.d  |  14 +++
 gas/testsuite/gas/riscv/option-arch-01b.d  |   8 ++
 gas/testsuite/gas/riscv/option-arch-02.d   |   8 ++
 gas/testsuite/gas/riscv/option-arch-02.s   |   8 ++
 gas/testsuite/gas/riscv/option-arch-03.d   |   8 ++
 gas/testsuite/gas/riscv/option-arch-03.s   |   3 +
 gas/testsuite/gas/riscv/option-arch-fail.d |   3 +
 gas/testsuite/gas/riscv/option-arch-fail.l |   8 ++
 gas/testsuite/gas/riscv/option-arch-fail.s |  10 ++
 14 files changed, 272 insertions(+), 43 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/option-arch-01.s
 create mode 100644 gas/testsuite/gas/riscv/option-arch-01a.d
 create mode 100644 gas/testsuite/gas/riscv/option-arch-01b.d
 create mode 100644 gas/testsuite/gas/riscv/option-arch-02.d
 create mode 100644 gas/testsuite/gas/riscv/option-arch-02.s
 create mode 100644 gas/testsuite/gas/riscv/option-arch-03.d
 create mode 100644 gas/testsuite/gas/riscv/option-arch-03.s
 create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.d
 create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.l
 create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.s

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 91afd4c..b8da40c 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1468,7 +1468,15 @@ riscv_add_subset (riscv_subset_list_t *subset_list,
   riscv_subset_t *current, *new;
 
   if (riscv_lookup_subset (subset_list, subset, &current))
-    return;
+    {
+      if (major != RISCV_UNKNOWN_VERSION
+	  && minor != RISCV_UNKNOWN_VERSION)
+	{
+	  current->major_version = major;
+	  current->minor_version = minor;
+	}
+      return;
+    }
 
   new = xmalloc (sizeof *new);
   new->name = xstrdup (subset);
@@ -2138,6 +2146,37 @@ riscv_arch_str (unsigned xlen, const riscv_subset_list_t *subset)
   return attr_str;
 }
 
+/* Copy the subset in the subset list.  */
+
+static struct riscv_subset_t *
+riscv_copy_subset (riscv_subset_list_t *subset_list,
+		   riscv_subset_t *subset)
+{
+  if (subset == NULL)
+    return NULL;
+
+  riscv_subset_t *new = xmalloc (sizeof *new);
+  new->name = xstrdup (subset->name);
+  new->major_version = subset->major_version;
+  new->minor_version = subset->minor_version;
+  new->next = riscv_copy_subset (subset_list, subset->next);
+
+  if (subset->next == NULL)
+    subset_list->tail = new;
+
+  return new;
+}
+
+/* Copy the subset list.  */
+
+riscv_subset_list_t *
+riscv_copy_subset_list (riscv_subset_list_t *subset_list)
+{
+  riscv_subset_list_t *new = xmalloc (sizeof *new);
+  new->head = riscv_copy_subset (new, subset_list->head);
+  return new;
+}
+
 /* Remove the SUBSET from the subset list.  */
 
 static void
@@ -2164,40 +2203,111 @@ riscv_remove_subset (riscv_subset_list_t *subset_list,
 }
 
 /* Add/Remove an extension to/from the subset list.  This is used for
-   the .option rvc or norvc.  */
+   the .option rvc or norvc, and .option arch directives.  */
 
 bool
 riscv_update_subset (riscv_parse_subset_t *rps,
-		     const char *subset,
-		     bool removed)
+		     const char *str)
 {
-  if (strlen (subset) == 0
-      || (strlen (subset) == 1
-	  && riscv_ext_order[(*subset - 'a')] == 0)
-      || (strlen (subset) > 1
-	  && rps->check_unknown_prefixed_ext
-	  && !riscv_recognized_prefixed_ext (subset)))
-    {
-      rps->error_handler
-	(_("riscv_update_subset: unknown ISA extension `%s'"), subset);
-      return false;
-    }
+  const char *p = str;
 
-  if (removed)
+  do
     {
-      if (strcmp (subset, "i") == 0)
+      int major_version = RISCV_UNKNOWN_VERSION;
+      int minor_version = RISCV_UNKNOWN_VERSION;
+
+      bool removed = false;
+      switch (*p++)
+	{
+	case '+': removed = false; break;
+	case '-': removed = true; break;
+	case '=':
+	  riscv_release_subset_list (rps->subset_list);
+	  return riscv_parse_subset (rps, p);
+	default:
+	  rps->error_handler
+	    (_("extensions must begin with +/-/= in .option arch `%s'"), str);
+	  return false;
+	}
+
+      char *subset = xstrdup (p);
+      char *q = subset;
+      const char *end_of_version;
+      /* Extract the whole prefixed extension by ','.  */
+      while (*q != '\0' && *q != ',')
+        q++;
+      /* Look forward to the first letter which is not <major>p<minor>.  */
+      bool find_any_version = false;
+      bool find_minor_version = false;
+      while (1)
+        {
+	  q--;
+	  if (ISDIGIT (*q))
+	    find_any_version = true;
+	  else if (find_any_version
+		   && !find_minor_version
+		   && *q == 'p'
+		   && ISDIGIT (*(q - 1)))
+	    find_minor_version = true;
+	  else
+	    break;
+	}
+      q++;
+      /* Check if the end of extension is 'p' or not.  If yes, then
+	 the second letter from the end cannot be number.  */
+      if (*(q - 1) == 'p' && ISDIGIT (*(q - 2)))
+	{
+	  *q = '\0';
+	  rps->error_handler
+	    (_("invalid ISA extension ends with <number>p "
+	       "in .option arch `%s'"), str);
+	  free (subset);
+	  return false;
+	}
+      end_of_version =
+	riscv_parsing_subset_version (q, &major_version, &minor_version);
+      *q = '\0';
+      if (end_of_version == NULL)
+	{
+	  free (subset);
+	  return false;
+	}
+
+      if (strlen (subset) == 0
+	  || (strlen (subset) == 1
+	      && riscv_ext_order[(*subset - 'a')] == 0)
+	  || (strlen (subset) > 1
+	      && rps->check_unknown_prefixed_ext
+	      && !riscv_recognized_prefixed_ext (subset)))
 	{
 	  rps->error_handler
-	    (_("riscv_update_subset: cannot remove extension i from "
-	       "the subset list"));
+	    (_("unknown ISA extension `%s' in .option arch `%s'"),
+	     subset, str);
+	  free (subset);
 	  return false;
 	}
-      riscv_remove_subset (rps->subset_list, subset);
+
+      if (removed)
+	{
+	  if (strcmp (subset, "i") == 0)
+	    {
+	      rps->error_handler
+		(_("cannot remove extension `i' in .option arch `%s'"), str);
+	      free (subset);
+	      return false;
+	    }
+	  riscv_remove_subset (rps->subset_list, subset);
+	}
+      else
+	riscv_parse_add_subset (rps, subset, major_version, minor_version, true);
+      p += end_of_version - subset;
+      free (subset);
     }
-  else
-    riscv_parse_add_subset (rps, subset,
-			    RISCV_UNKNOWN_VERSION,
-			    RISCV_UNKNOWN_VERSION, true);
+  while (*p++ == ',');
+
+  if (*(--p) != '\0')
+    rps->error_handler
+      (_("unexpected value in .option arch `%s'"), str);
 
   riscv_parse_add_implicit_subsets (rps);
   return riscv_parse_check_conflicts (rps);
diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
index 8de9adc..ea7edc4 100644
--- a/bfd/elfxx-riscv.h
+++ b/bfd/elfxx-riscv.h
@@ -92,8 +92,11 @@ riscv_estimate_digit (unsigned);
 extern int
 riscv_compare_subsets (const char *, const char *);
 
+extern riscv_subset_list_t *
+riscv_copy_subset_list (riscv_subset_list_t *);
+
 extern bool
-riscv_update_subset (riscv_parse_subset_t *, const char *, bool);
+riscv_update_subset (riscv_parse_subset_t *, const char *);
 
 extern bool
 riscv_subset_supports (riscv_parse_subset_t *, const char *);
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 930cfb2..da3d911 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -235,16 +235,27 @@ riscv_set_rvc (bool rvc_value)
    the architecture string.  The architecture string can be set by the
    -march option, the elf architecture attributes, and the --with-arch
    configure option.  */
-static riscv_subset_list_t riscv_subsets;
+static riscv_subset_list_t *riscv_subsets = NULL;
 static riscv_parse_subset_t riscv_rps_as =
 {
-  &riscv_subsets,	/* subset_list.  */
+  NULL,			/* subset_list, we will set it later once
+			   riscv_opts_stack is created or updated.  */
   as_bad,		/* error_handler.  */
   &xlen,		/* xlen.  */
   &default_isa_spec,	/* isa_spec.  */
   true,			/* check_unknown_prefixed_ext.  */
 };
 
+/* This structure is used to hold a stack of .option values.  */
+struct riscv_option_stack
+{
+  struct riscv_option_stack *next;
+  struct riscv_set_options options;
+  riscv_subset_list_t *subset_list;
+};
+
+static struct riscv_option_stack *riscv_opts_stack = NULL;
+
 /* Set which ISA and extensions are available.  */
 
 static void
@@ -257,7 +268,14 @@ riscv_set_arch (const char *s)
       return;
     }
 
-  riscv_release_subset_list (&riscv_subsets);
+  if (riscv_subsets == NULL)
+    {
+      riscv_subsets = XNEW (riscv_subset_list_t);
+      riscv_subsets->head = NULL;
+      riscv_subsets->tail = NULL;
+      riscv_rps_as.subset_list = riscv_subsets;
+    }
+  riscv_release_subset_list (riscv_subsets);
   riscv_parse_subset (&riscv_rps_as, s);
 
   riscv_set_rvc (false);
@@ -3715,15 +3733,6 @@ riscv_pre_output_hook (void)
   subseg_set (seg, subseg);
 }
 
-/* This structure is used to hold a stack of .option values.  */
-struct riscv_option_stack
-{
-  struct riscv_option_stack *next;
-  struct riscv_set_options options;
-};
-
-static struct riscv_option_stack *riscv_opts_stack;
-
 /* Handle the .option pseudo-op.  */
 
 static void
@@ -3738,12 +3747,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
 
   if (strcmp (name, "rvc") == 0)
     {
-      riscv_update_subset (&riscv_rps_as, "c", false);
+      riscv_update_subset (&riscv_rps_as, "+c");
       riscv_set_rvc (true);
     }
   else if (strcmp (name, "norvc") == 0)
     {
-      riscv_update_subset (&riscv_rps_as, "c", true);
+      riscv_update_subset (&riscv_rps_as, "-c");
       riscv_set_rvc (false);
     }
   else if (strcmp (name, "pic") == 0)
@@ -3758,14 +3767,24 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
     riscv_opts.csr_check = true;
   else if (strcmp (name, "no-csr-check") == 0)
     riscv_opts.csr_check = false;
+  else if (strncmp (name, "arch,", 5) == 0)
+    {
+      name += 5;
+      if (ISSPACE (*name) && *name != '\0')
+	name++;
+      riscv_update_subset (&riscv_rps_as, name);
+    }
   else if (strcmp (name, "push") == 0)
     {
       struct riscv_option_stack *s;
 
-      s = (struct riscv_option_stack *) xmalloc (sizeof *s);
+      s = XNEW (struct riscv_option_stack);
       s->next = riscv_opts_stack;
       s->options = riscv_opts;
+      s->subset_list = riscv_subsets;
       riscv_opts_stack = s;
+      riscv_subsets = riscv_copy_subset_list (s->subset_list);
+      riscv_rps_as.subset_list = riscv_subsets;
     }
   else if (strcmp (name, "pop") == 0)
     {
@@ -3776,8 +3795,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
 	as_bad (_(".option pop with no .option push"));
       else
 	{
-	  riscv_opts = s->options;
+	  riscv_subset_list_t *release_subsets = riscv_subsets;
 	  riscv_opts_stack = s->next;
+	  riscv_opts = s->options;
+	  riscv_subsets = s->subset_list;
+	  riscv_rps_as.subset_list = riscv_subsets;
+	  riscv_release_subset_list (release_subsets);
 	  free (s);
 	}
     }
@@ -4262,7 +4285,7 @@ riscv_write_out_attrs (void)
   unsigned int i;
 
   /* Re-write architecture elf attribute.  */
-  arch_str = riscv_arch_str (xlen, &riscv_subsets);
+  arch_str = riscv_arch_str (xlen, riscv_subsets);
   bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str);
   xfree ((void *) arch_str);
 
diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
index bfbf61d..697be3a 100644
--- a/gas/doc/c-riscv.texi
+++ b/gas/doc/c-riscv.texi
@@ -194,7 +194,7 @@ command-line options are respected for the bulk of the file being assembled.
 @itemx norvc
 Enables or disables the generation of compressed instructions.  Instructions
 are opportunistically compressed by the RISC-V assembler when possible, but
-sometimes this behavior is not desirable.
+sometimes this behavior is not desirable, especially when handling alignments.
 
 @item pic
 @itemx nopic
@@ -212,6 +212,19 @@ desirable.
 @itemx no-csr-check
 Enables or disables the CSR checking.
 
+@item arch, @var{+extension[version]} [,...,@var{+extension_n[version_n]}]
+@itemx arch, @var{-extension} [,...,@var{-extension_n}]
+@itemx arch, @var{=ISA}
+Enables or disables the extensions for specific code region.  For example,
+@samp{.option arch, +m2p0} means add m extension with version 2.0, and
+@samp{.option arch, -f, -d} means remove extensions, f and d, from the
+architecture string.  Note that, @samp{.option arch, +c, -c} have the same
+behavior as @samp{.option rvc, norvc}.  However, they are also undesirable
+sometimes.  Besides, @samp{.option arch, -i} is illegal, since we cannot
+remove the base i extension anytime.  If you want to reset the whole ISA
+string, you can also use @samp{.option arch, =rv32imac} to overwrite the
+previous settings.
+
 @cindex INSN directives
 @item .insn @var{type}, @var{operand} [,...,@var{operand_n}]
 @itemx .insn @var{insn_length}, @var{value}
diff --git a/gas/testsuite/gas/riscv/option-arch-01.s b/gas/testsuite/gas/riscv/option-arch-01.s
new file mode 100644
index 0000000..201f9b3
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-01.s
@@ -0,0 +1,10 @@
+.attribute arch, "rv64ic"
+add	a0, a0, a1
+.option push
+.option arch, +d2p0, -c, +xvendor1p0
+add	a0, a0, a1
+frcsr	a0	# Should add mapping symbol with ISA here, and then dump it to frcsr.
+.option push
+.option arch, +i3p0, +m3p0, +d3p0
+.option pop
+.option pop
diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
new file mode 100644
index 0000000..59bc1d2
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-01a.d
@@ -0,0 +1,14 @@
+#as:
+#source: option-arch-01.s
+#objdump: -d
+
+.*:[   ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+[0-9a-f]+:[  	]+952e[    	]+add[        	]+a0,a0,a1
+[ 	]+[0-9a-f]+:[  	]+00b50533[    	]+add[        	]+a0,a0,a1
+[ 	]+[0-9a-f]+:[  	]+00302573[    	]+csrr[        	]+a0,fcsr
+#...
diff --git a/gas/testsuite/gas/riscv/option-arch-01b.d b/gas/testsuite/gas/riscv/option-arch-01b.d
new file mode 100644
index 0000000..9a6c2c5
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-01b.d
@@ -0,0 +1,8 @@
+#as:
+#readelf: -A
+#source: option-arch-01.s
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: "rv64i2p0_c2p0"
+#...
diff --git a/gas/testsuite/gas/riscv/option-arch-02.d b/gas/testsuite/gas/riscv/option-arch-02.d
new file mode 100644
index 0000000..0fe89ec
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-02.d
@@ -0,0 +1,8 @@
+#as:
+#readelf: -A
+#source: option-arch-02.s
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: "rv64i3p0_m3p0_f2p0_d3p0_c2p0_xvendor32x3p0"
+#...
diff --git a/gas/testsuite/gas/riscv/option-arch-02.s b/gas/testsuite/gas/riscv/option-arch-02.s
new file mode 100644
index 0000000..f4ceee84
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-02.s
@@ -0,0 +1,8 @@
+.attribute arch, "rv64ic"
+add	a0, a0, a1
+.option push
+.option arch, +d2p0, -c, +xvendor1p0
+add	a0, a0, a1
+frcsr	a0
+.option pop
+.option arch, +i3p0, +m3p0, +d3p0, +xvendor32x3p0
diff --git a/gas/testsuite/gas/riscv/option-arch-03.d b/gas/testsuite/gas/riscv/option-arch-03.d
new file mode 100644
index 0000000..b621d03
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-03.d
@@ -0,0 +1,8 @@
+#as:
+#readelf: -A
+#source: option-arch-03.s
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: "rv32i2p0_c2p0"
+#...
diff --git a/gas/testsuite/gas/riscv/option-arch-03.s b/gas/testsuite/gas/riscv/option-arch-03.s
new file mode 100644
index 0000000..7183140
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-03.s
@@ -0,0 +1,3 @@
+.attribute arch, "rv64ic"
+.option arch, +d2p0, -c
+.option arch, =rv32ic
diff --git a/gas/testsuite/gas/riscv/option-arch-fail.d b/gas/testsuite/gas/riscv/option-arch-fail.d
new file mode 100644
index 0000000..bce5d32
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-fail.d
@@ -0,0 +1,3 @@
+#as:
+#source: option-arch-fail.s
+#error_output: option-arch-fail.l
diff --git a/gas/testsuite/gas/riscv/option-arch-fail.l b/gas/testsuite/gas/riscv/option-arch-fail.l
new file mode 100644
index 0000000..3e0599e
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-fail.l
@@ -0,0 +1,8 @@
+.*Assembler messages:
+.*Error: extensions must begin with \+/\-/\= in .option arch `m2p0'
+.*Error: cannot remove extension `i' in .option arch `\-i'
+.*Error: unknown ISA extension `zsubset' in .option arch `\+zsubset2p0'
+.*Error: unknown ISA extension `f2p0_d' in .option arch `\+f2p0_d2p0'
+.*Error: unknown ISA extension `' in .option arch `\+'
+.*Error: invalid ISA extension ends with <number>p in .option arch `\+xvendor2p'
+.*Error: .option pop with no .option push
diff --git a/gas/testsuite/gas/riscv/option-arch-fail.s b/gas/testsuite/gas/riscv/option-arch-fail.s
new file mode 100644
index 0000000..a0b1bde
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-fail.s
@@ -0,0 +1,10 @@
+.attribute arch, "rv64ic"
+.option push
+.option arch, m2p0
+.option arch, -i
+.option arch, +zsubset2p0
+.option arch, +f2p0_d2p0
+.option arch, +
+.option arch, +xvendor2p
+.option pop
+.option pop
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [committed v3] RISC-V: Support new .option arch directive.
  2021-11-19 10:53 [committed v3] RISC-V: Support new .option arch directive Nelson Chu
@ 2021-11-19 16:01 ` Palmer Dabbelt
  2021-11-20  2:17   ` Nelson Chu
  0 siblings, 1 reply; 4+ messages in thread
From: Palmer Dabbelt @ 2021-11-19 16:01 UTC (permalink / raw)
  To: Nelson Chu; +Cc: binutils, Jim Wilson, kito.cheng, Nelson Chu

On Fri, 19 Nov 2021 02:53:53 PST (-0800), Nelson Chu wrote:
> https://github.com/riscv/riscv-asm-manual/pull/67
>
> Format:
> .option arch, +<extension><version>, ...
> .option arch, -<extension>
> .option arch, =<ISA string>
>
> The new direcitve is used to enable/disable extensions for the specific
> code region.  For example,
>
> .attribute arch, "rv64ic"   # arch = rv64i2p0_c2p0
> .option push
> .option arch, +d2p0, -c     # arch = rv64i2p0_f2p0_d2p0, f is added implied
> .option arch, =rv32gc       # arch = rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0
> .option pop                 # arch = rv64i2p0_c2p0
>
> Note that,
> 1. ".option rvc/norvc" have the same behavior as ".option arch +c/-c".
> 2. ".option arch -i" is illegal, since we cannot remove base i extension.

Should that apply to E as well?  RV32E is as much of a base ISA as RV32I 
is, at least according to the specs, but this allows something like

   .attribute arch, "rv32e"
   .option arch, -e
   add x31, x31, x31

to turn E into I.

It does feel kind of nice to treat E as an extension here, as that would 
allow us to mix in short E-compatible code segments (to try and detect 
the base ISA, for example), but the ISA doesn't treat it that way so 
it's kind of odd to do so in the toolchain.

I'm not really sure I have a case where "-e" is any better than 
"=rv32e", though, so maybe it's better to just play it save and reject 
+/-e?

> 3. If arch=rv64i2p0, then ".option arch, +i3p0" will update the i's version
>    from 2.0 to 3.0.
> 4. If arch=rv64i3p0, then ".option arch, +i" will update the i's version
>    from 2.0 to the default one according to the chosen isa spec.
>
> bfd/
> 	* elfxx-riscv.c (riscv_add_subset): If the subset is already added,
> 	and the new versions are not RISCV_UNKNOWN_VERSION, then update the
> 	versions to the subset list.
> 	(riscv_copy_subset): New function.  Copy the subset from list.
> 	(riscv_copy_subset_list): New function.  Return the new copyed list.
> 	(riscv_update_subset): Updated to make .option arch directives workable.
> 	* elfxx-riscv.h: Updated.
> gas/
> 	* config/tc-riscv.c (riscv_subsets): Defined as a pointer.
> 	(riscv_rps_as): Init the subset_list to NULL, we will set it later
> 	once riscv_opts_stack is created or updated.
> 	(struct riscv_option_stack, riscv_opts_stack): Moved forward.
> 	(riscv_set_arch): Updated.
> 	(s_riscv_option): Support new .option arch directive, to add, remove
> 	or update subsets for the specific code region.
> 	(riscv_write_out_attrs): Updated.
> 	* doc/c-riscv.texi: Added document for new .option arch directive.
> 	* testsuite/gas/riscv/option-arch-01a.d: New testcase.
> 	* testsuite/gas/riscv/option-arch-01b.d: Likewise.
> 	* testsuite/gas/riscv/option-arch-01.s: Likewise..
> 	* testsuite/gas/riscv/option-arch-02.d: Likewise.
> 	* testsuite/gas/riscv/option-arch-02.s: Likewise.
> 	* testsuite/gas/riscv/option-arch-fail.d: Likewise.
> 	* testsuite/gas/riscv/option-arch-fail.l: Likewise.
> 	* testsuite/gas/riscv/option-arch-fail.s: Likewise.
> ---
>  bfd/elfxx-riscv.c                          | 158 ++++++++++++++++++++++++-----
>  bfd/elfxx-riscv.h                          |   5 +-
>  gas/config/tc-riscv.c                      |  57 +++++++----
>  gas/doc/c-riscv.texi                       |  15 ++-
>  gas/testsuite/gas/riscv/option-arch-01.s   |  10 ++
>  gas/testsuite/gas/riscv/option-arch-01a.d  |  14 +++
>  gas/testsuite/gas/riscv/option-arch-01b.d  |   8 ++
>  gas/testsuite/gas/riscv/option-arch-02.d   |   8 ++
>  gas/testsuite/gas/riscv/option-arch-02.s   |   8 ++
>  gas/testsuite/gas/riscv/option-arch-03.d   |   8 ++
>  gas/testsuite/gas/riscv/option-arch-03.s   |   3 +
>  gas/testsuite/gas/riscv/option-arch-fail.d |   3 +
>  gas/testsuite/gas/riscv/option-arch-fail.l |   8 ++
>  gas/testsuite/gas/riscv/option-arch-fail.s |  10 ++
>  14 files changed, 272 insertions(+), 43 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-01.s
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-01a.d
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-01b.d
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-02.d
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-02.s
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-03.d
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-03.s
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.s
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 91afd4c..b8da40c 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1468,7 +1468,15 @@ riscv_add_subset (riscv_subset_list_t *subset_list,
>    riscv_subset_t *current, *new;
>
>    if (riscv_lookup_subset (subset_list, subset, &current))
> -    return;
> +    {
> +      if (major != RISCV_UNKNOWN_VERSION
> +	  && minor != RISCV_UNKNOWN_VERSION)
> +	{
> +	  current->major_version = major;
> +	  current->minor_version = minor;
> +	}
> +      return;
> +    }
>
>    new = xmalloc (sizeof *new);
>    new->name = xstrdup (subset);
> @@ -2138,6 +2146,37 @@ riscv_arch_str (unsigned xlen, const riscv_subset_list_t *subset)
>    return attr_str;
>  }
>
> +/* Copy the subset in the subset list.  */
> +
> +static struct riscv_subset_t *
> +riscv_copy_subset (riscv_subset_list_t *subset_list,
> +		   riscv_subset_t *subset)
> +{
> +  if (subset == NULL)
> +    return NULL;
> +
> +  riscv_subset_t *new = xmalloc (sizeof *new);
> +  new->name = xstrdup (subset->name);
> +  new->major_version = subset->major_version;
> +  new->minor_version = subset->minor_version;
> +  new->next = riscv_copy_subset (subset_list, subset->next);
> +
> +  if (subset->next == NULL)
> +    subset_list->tail = new;
> +
> +  return new;
> +}
> +
> +/* Copy the subset list.  */
> +
> +riscv_subset_list_t *
> +riscv_copy_subset_list (riscv_subset_list_t *subset_list)
> +{
> +  riscv_subset_list_t *new = xmalloc (sizeof *new);
> +  new->head = riscv_copy_subset (new, subset_list->head);
> +  return new;
> +}
> +
>  /* Remove the SUBSET from the subset list.  */
>
>  static void
> @@ -2164,40 +2203,111 @@ riscv_remove_subset (riscv_subset_list_t *subset_list,
>  }
>
>  /* Add/Remove an extension to/from the subset list.  This is used for
> -   the .option rvc or norvc.  */
> +   the .option rvc or norvc, and .option arch directives.  */
>
>  bool
>  riscv_update_subset (riscv_parse_subset_t *rps,
> -		     const char *subset,
> -		     bool removed)
> +		     const char *str)
>  {
> -  if (strlen (subset) == 0
> -      || (strlen (subset) == 1
> -	  && riscv_ext_order[(*subset - 'a')] == 0)
> -      || (strlen (subset) > 1
> -	  && rps->check_unknown_prefixed_ext
> -	  && !riscv_recognized_prefixed_ext (subset)))
> -    {
> -      rps->error_handler
> -	(_("riscv_update_subset: unknown ISA extension `%s'"), subset);
> -      return false;
> -    }
> +  const char *p = str;
>
> -  if (removed)
> +  do
>      {
> -      if (strcmp (subset, "i") == 0)
> +      int major_version = RISCV_UNKNOWN_VERSION;
> +      int minor_version = RISCV_UNKNOWN_VERSION;
> +
> +      bool removed = false;
> +      switch (*p++)
> +	{
> +	case '+': removed = false; break;
> +	case '-': removed = true; break;
> +	case '=':
> +	  riscv_release_subset_list (rps->subset_list);
> +	  return riscv_parse_subset (rps, p);
> +	default:
> +	  rps->error_handler
> +	    (_("extensions must begin with +/-/= in .option arch `%s'"), str);
> +	  return false;
> +	}
> +
> +      char *subset = xstrdup (p);
> +      char *q = subset;
> +      const char *end_of_version;
> +      /* Extract the whole prefixed extension by ','.  */
> +      while (*q != '\0' && *q != ',')
> +        q++;
> +      /* Look forward to the first letter which is not <major>p<minor>.  */
> +      bool find_any_version = false;
> +      bool find_minor_version = false;
> +      while (1)
> +        {
> +	  q--;
> +	  if (ISDIGIT (*q))
> +	    find_any_version = true;
> +	  else if (find_any_version
> +		   && !find_minor_version
> +		   && *q == 'p'
> +		   && ISDIGIT (*(q - 1)))
> +	    find_minor_version = true;
> +	  else
> +	    break;
> +	}
> +      q++;
> +      /* Check if the end of extension is 'p' or not.  If yes, then
> +	 the second letter from the end cannot be number.  */
> +      if (*(q - 1) == 'p' && ISDIGIT (*(q - 2)))
> +	{
> +	  *q = '\0';
> +	  rps->error_handler
> +	    (_("invalid ISA extension ends with <number>p "
> +	       "in .option arch `%s'"), str);
> +	  free (subset);
> +	  return false;
> +	}
> +      end_of_version =
> +	riscv_parsing_subset_version (q, &major_version, &minor_version);
> +      *q = '\0';
> +      if (end_of_version == NULL)
> +	{
> +	  free (subset);
> +	  return false;
> +	}
> +
> +      if (strlen (subset) == 0
> +	  || (strlen (subset) == 1
> +	      && riscv_ext_order[(*subset - 'a')] == 0)
> +	  || (strlen (subset) > 1
> +	      && rps->check_unknown_prefixed_ext
> +	      && !riscv_recognized_prefixed_ext (subset)))
>  	{
>  	  rps->error_handler
> -	    (_("riscv_update_subset: cannot remove extension i from "
> -	       "the subset list"));
> +	    (_("unknown ISA extension `%s' in .option arch `%s'"),
> +	     subset, str);
> +	  free (subset);
>  	  return false;
>  	}
> -      riscv_remove_subset (rps->subset_list, subset);
> +
> +      if (removed)
> +	{
> +	  if (strcmp (subset, "i") == 0)
> +	    {
> +	      rps->error_handler
> +		(_("cannot remove extension `i' in .option arch `%s'"), str);
> +	      free (subset);
> +	      return false;
> +	    }
> +	  riscv_remove_subset (rps->subset_list, subset);
> +	}
> +      else
> +	riscv_parse_add_subset (rps, subset, major_version, minor_version, true);
> +      p += end_of_version - subset;
> +      free (subset);
>      }
> -  else
> -    riscv_parse_add_subset (rps, subset,
> -			    RISCV_UNKNOWN_VERSION,
> -			    RISCV_UNKNOWN_VERSION, true);
> +  while (*p++ == ',');
> +
> +  if (*(--p) != '\0')
> +    rps->error_handler
> +      (_("unexpected value in .option arch `%s'"), str);
>
>    riscv_parse_add_implicit_subsets (rps);
>    return riscv_parse_check_conflicts (rps);
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index 8de9adc..ea7edc4 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -92,8 +92,11 @@ riscv_estimate_digit (unsigned);
>  extern int
>  riscv_compare_subsets (const char *, const char *);
>
> +extern riscv_subset_list_t *
> +riscv_copy_subset_list (riscv_subset_list_t *);
> +
>  extern bool
> -riscv_update_subset (riscv_parse_subset_t *, const char *, bool);
> +riscv_update_subset (riscv_parse_subset_t *, const char *);
>
>  extern bool
>  riscv_subset_supports (riscv_parse_subset_t *, const char *);
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 930cfb2..da3d911 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -235,16 +235,27 @@ riscv_set_rvc (bool rvc_value)
>     the architecture string.  The architecture string can be set by the
>     -march option, the elf architecture attributes, and the --with-arch
>     configure option.  */
> -static riscv_subset_list_t riscv_subsets;
> +static riscv_subset_list_t *riscv_subsets = NULL;
>  static riscv_parse_subset_t riscv_rps_as =
>  {
> -  &riscv_subsets,	/* subset_list.  */
> +  NULL,			/* subset_list, we will set it later once
> +			   riscv_opts_stack is created or updated.  */
>    as_bad,		/* error_handler.  */
>    &xlen,		/* xlen.  */
>    &default_isa_spec,	/* isa_spec.  */
>    true,			/* check_unknown_prefixed_ext.  */
>  };
>
> +/* This structure is used to hold a stack of .option values.  */
> +struct riscv_option_stack
> +{
> +  struct riscv_option_stack *next;
> +  struct riscv_set_options options;
> +  riscv_subset_list_t *subset_list;
> +};
> +
> +static struct riscv_option_stack *riscv_opts_stack = NULL;
> +
>  /* Set which ISA and extensions are available.  */
>
>  static void
> @@ -257,7 +268,14 @@ riscv_set_arch (const char *s)
>        return;
>      }
>
> -  riscv_release_subset_list (&riscv_subsets);
> +  if (riscv_subsets == NULL)
> +    {
> +      riscv_subsets = XNEW (riscv_subset_list_t);
> +      riscv_subsets->head = NULL;
> +      riscv_subsets->tail = NULL;
> +      riscv_rps_as.subset_list = riscv_subsets;
> +    }
> +  riscv_release_subset_list (riscv_subsets);
>    riscv_parse_subset (&riscv_rps_as, s);
>
>    riscv_set_rvc (false);
> @@ -3715,15 +3733,6 @@ riscv_pre_output_hook (void)
>    subseg_set (seg, subseg);
>  }
>
> -/* This structure is used to hold a stack of .option values.  */
> -struct riscv_option_stack
> -{
> -  struct riscv_option_stack *next;
> -  struct riscv_set_options options;
> -};
> -
> -static struct riscv_option_stack *riscv_opts_stack;
> -
>  /* Handle the .option pseudo-op.  */
>
>  static void
> @@ -3738,12 +3747,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>
>    if (strcmp (name, "rvc") == 0)
>      {
> -      riscv_update_subset (&riscv_rps_as, "c", false);
> +      riscv_update_subset (&riscv_rps_as, "+c");
>        riscv_set_rvc (true);
>      }
>    else if (strcmp (name, "norvc") == 0)
>      {
> -      riscv_update_subset (&riscv_rps_as, "c", true);
> +      riscv_update_subset (&riscv_rps_as, "-c");
>        riscv_set_rvc (false);
>      }
>    else if (strcmp (name, "pic") == 0)
> @@ -3758,14 +3767,24 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>      riscv_opts.csr_check = true;
>    else if (strcmp (name, "no-csr-check") == 0)
>      riscv_opts.csr_check = false;
> +  else if (strncmp (name, "arch,", 5) == 0)
> +    {
> +      name += 5;
> +      if (ISSPACE (*name) && *name != '\0')
> +	name++;
> +      riscv_update_subset (&riscv_rps_as, name);
> +    }
>    else if (strcmp (name, "push") == 0)
>      {
>        struct riscv_option_stack *s;
>
> -      s = (struct riscv_option_stack *) xmalloc (sizeof *s);
> +      s = XNEW (struct riscv_option_stack);
>        s->next = riscv_opts_stack;
>        s->options = riscv_opts;
> +      s->subset_list = riscv_subsets;
>        riscv_opts_stack = s;
> +      riscv_subsets = riscv_copy_subset_list (s->subset_list);
> +      riscv_rps_as.subset_list = riscv_subsets;
>      }
>    else if (strcmp (name, "pop") == 0)
>      {
> @@ -3776,8 +3795,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>  	as_bad (_(".option pop with no .option push"));
>        else
>  	{
> -	  riscv_opts = s->options;
> +	  riscv_subset_list_t *release_subsets = riscv_subsets;
>  	  riscv_opts_stack = s->next;
> +	  riscv_opts = s->options;
> +	  riscv_subsets = s->subset_list;
> +	  riscv_rps_as.subset_list = riscv_subsets;
> +	  riscv_release_subset_list (release_subsets);
>  	  free (s);
>  	}
>      }
> @@ -4262,7 +4285,7 @@ riscv_write_out_attrs (void)
>    unsigned int i;
>
>    /* Re-write architecture elf attribute.  */
> -  arch_str = riscv_arch_str (xlen, &riscv_subsets);
> +  arch_str = riscv_arch_str (xlen, riscv_subsets);
>    bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str);
>    xfree ((void *) arch_str);
>
> diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
> index bfbf61d..697be3a 100644
> --- a/gas/doc/c-riscv.texi
> +++ b/gas/doc/c-riscv.texi
> @@ -194,7 +194,7 @@ command-line options are respected for the bulk of the file being assembled.
>  @itemx norvc
>  Enables or disables the generation of compressed instructions.  Instructions
>  are opportunistically compressed by the RISC-V assembler when possible, but
> -sometimes this behavior is not desirable.
> +sometimes this behavior is not desirable, especially when handling alignments.
>
>  @item pic
>  @itemx nopic
> @@ -212,6 +212,19 @@ desirable.
>  @itemx no-csr-check
>  Enables or disables the CSR checking.
>
> +@item arch, @var{+extension[version]} [,...,@var{+extension_n[version_n]}]
> +@itemx arch, @var{-extension} [,...,@var{-extension_n}]
> +@itemx arch, @var{=ISA}
> +Enables or disables the extensions for specific code region.  For example,
> +@samp{.option arch, +m2p0} means add m extension with version 2.0, and
> +@samp{.option arch, -f, -d} means remove extensions, f and d, from the
> +architecture string.  Note that, @samp{.option arch, +c, -c} have the same
> +behavior as @samp{.option rvc, norvc}.  However, they are also undesirable
> +sometimes.  Besides, @samp{.option arch, -i} is illegal, since we cannot
> +remove the base i extension anytime.  If you want to reset the whole ISA
> +string, you can also use @samp{.option arch, =rv32imac} to overwrite the
> +previous settings.
> +
>  @cindex INSN directives
>  @item .insn @var{type}, @var{operand} [,...,@var{operand_n}]
>  @itemx .insn @var{insn_length}, @var{value}
> diff --git a/gas/testsuite/gas/riscv/option-arch-01.s b/gas/testsuite/gas/riscv/option-arch-01.s
> new file mode 100644
> index 0000000..201f9b3
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-01.s
> @@ -0,0 +1,10 @@
> +.attribute arch, "rv64ic"
> +add	a0, a0, a1
> +.option push
> +.option arch, +d2p0, -c, +xvendor1p0
> +add	a0, a0, a1
> +frcsr	a0	# Should add mapping symbol with ISA here, and then dump it to frcsr.
> +.option push
> +.option arch, +i3p0, +m3p0, +d3p0
> +.option pop
> +.option pop
> diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
> new file mode 100644
> index 0000000..59bc1d2
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-01a.d
> @@ -0,0 +1,14 @@
> +#as:
> +#source: option-arch-01.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[ 	]+[0-9a-f]+:[  	]+952e[    	]+add[        	]+a0,a0,a1
> +[ 	]+[0-9a-f]+:[  	]+00b50533[    	]+add[        	]+a0,a0,a1
> +[ 	]+[0-9a-f]+:[  	]+00302573[    	]+csrr[        	]+a0,fcsr
> +#...
> diff --git a/gas/testsuite/gas/riscv/option-arch-01b.d b/gas/testsuite/gas/riscv/option-arch-01b.d
> new file mode 100644
> index 0000000..9a6c2c5
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-01b.d
> @@ -0,0 +1,8 @@
> +#as:
> +#readelf: -A
> +#source: option-arch-01.s
> +
> +Attribute Section: riscv
> +File Attributes
> +  Tag_RISCV_arch: "rv64i2p0_c2p0"
> +#...
> diff --git a/gas/testsuite/gas/riscv/option-arch-02.d b/gas/testsuite/gas/riscv/option-arch-02.d
> new file mode 100644
> index 0000000..0fe89ec
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-02.d
> @@ -0,0 +1,8 @@
> +#as:
> +#readelf: -A
> +#source: option-arch-02.s
> +
> +Attribute Section: riscv
> +File Attributes
> +  Tag_RISCV_arch: "rv64i3p0_m3p0_f2p0_d3p0_c2p0_xvendor32x3p0"
> +#...
> diff --git a/gas/testsuite/gas/riscv/option-arch-02.s b/gas/testsuite/gas/riscv/option-arch-02.s
> new file mode 100644
> index 0000000..f4ceee84
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-02.s
> @@ -0,0 +1,8 @@
> +.attribute arch, "rv64ic"
> +add	a0, a0, a1
> +.option push
> +.option arch, +d2p0, -c, +xvendor1p0
> +add	a0, a0, a1
> +frcsr	a0
> +.option pop
> +.option arch, +i3p0, +m3p0, +d3p0, +xvendor32x3p0
> diff --git a/gas/testsuite/gas/riscv/option-arch-03.d b/gas/testsuite/gas/riscv/option-arch-03.d
> new file mode 100644
> index 0000000..b621d03
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-03.d
> @@ -0,0 +1,8 @@
> +#as:
> +#readelf: -A
> +#source: option-arch-03.s
> +
> +Attribute Section: riscv
> +File Attributes
> +  Tag_RISCV_arch: "rv32i2p0_c2p0"
> +#...
> diff --git a/gas/testsuite/gas/riscv/option-arch-03.s b/gas/testsuite/gas/riscv/option-arch-03.s
> new file mode 100644
> index 0000000..7183140
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-03.s
> @@ -0,0 +1,3 @@
> +.attribute arch, "rv64ic"
> +.option arch, +d2p0, -c
> +.option arch, =rv32ic
> diff --git a/gas/testsuite/gas/riscv/option-arch-fail.d b/gas/testsuite/gas/riscv/option-arch-fail.d
> new file mode 100644
> index 0000000..bce5d32
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-fail.d
> @@ -0,0 +1,3 @@
> +#as:
> +#source: option-arch-fail.s
> +#error_output: option-arch-fail.l
> diff --git a/gas/testsuite/gas/riscv/option-arch-fail.l b/gas/testsuite/gas/riscv/option-arch-fail.l
> new file mode 100644
> index 0000000..3e0599e
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-fail.l
> @@ -0,0 +1,8 @@
> +.*Assembler messages:
> +.*Error: extensions must begin with \+/\-/\= in .option arch `m2p0'
> +.*Error: cannot remove extension `i' in .option arch `\-i'
> +.*Error: unknown ISA extension `zsubset' in .option arch `\+zsubset2p0'
> +.*Error: unknown ISA extension `f2p0_d' in .option arch `\+f2p0_d2p0'
> +.*Error: unknown ISA extension `' in .option arch `\+'
> +.*Error: invalid ISA extension ends with <number>p in .option arch `\+xvendor2p'
> +.*Error: .option pop with no .option push
> diff --git a/gas/testsuite/gas/riscv/option-arch-fail.s b/gas/testsuite/gas/riscv/option-arch-fail.s
> new file mode 100644
> index 0000000..a0b1bde
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-fail.s
> @@ -0,0 +1,10 @@
> +.attribute arch, "rv64ic"
> +.option push
> +.option arch, m2p0
> +.option arch, -i
> +.option arch, +zsubset2p0
> +.option arch, +f2p0_d2p0
> +.option arch, +
> +.option arch, +xvendor2p
> +.option pop
> +.option pop

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [committed v3] RISC-V: Support new .option arch directive.
  2021-11-19 16:01 ` Palmer Dabbelt
@ 2021-11-20  2:17   ` Nelson Chu
  2021-11-20  2:32     ` Jessica Clarke
  0 siblings, 1 reply; 4+ messages in thread
From: Nelson Chu @ 2021-11-20  2:17 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: James Clarke, Jim Wilson, binutils, kito.cheng

> On Fri, 19 Nov 2021 02:53:53 PST (-0800), Nelson Chu wrote:
> > https://github.com/riscv/riscv-asm-manual/pull/67
> >
> > Format:
> > .option arch, +<extension><version>, ...
> > .option arch, -<extension>
> > .option arch, =<ISA string>
> >
> > The new direcitve is used to enable/disable extensions for the specific
> > code region.  For example,
> >
> > .attribute arch, "rv64ic"   # arch = rv64i2p0_c2p0
> > .option push
> > .option arch, +d2p0, -c     # arch = rv64i2p0_f2p0_d2p0, f is added
> implied
> > .option arch, =rv32gc       # arch = rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0
> > .option pop                 # arch = rv64i2p0_c2p0
> >
> > Note that,
> > 1. ".option rvc/norvc" have the same behavior as ".option arch +c/-c".
> > 2. ".option arch -i" is illegal, since we cannot remove base i extension.
>
> Should that apply to E as well?  RV32E is as much of a base ISA as RV32I
> is, at least according to the specs, but this allows something like
>
>    .attribute arch, "rv32e"
>    .option arch, -e
>    add x31, x31, x31
>
> to turn E into I.
>
> It does feel kind of nice to treat E as an extension here, as that would
> allow us to mix in short E-compatible code segments (to try and detect
> the base ISA, for example), but the ISA doesn't treat it that way so
> it's kind of odd to do so in the toolchain.
>
> I'm not really sure I have a case where "-e" is any better than
> "=rv32e", though, so maybe it's better to just play it save and reject
> +/-e?


This make sense to me, +- base extension looks weird, and we probably also
need to reject G extension when using both the operation +-.  Instead, if
users want to add or update the version of base extensions, which are I, E
and G, then they should use the operation = and reset the whole
architecture string.

Hi Kito, Hi Jessica,

Does this also make sense to you?  It would be great if this work for you
and could update to the psabi pr. Or maybe you have a better idea :)

Thank you very much
Nelson


> > 3. If arch=rv64i2p0, then ".option arch, +i3p0" will update the i's
> version
> >    from 2.0 to 3.0.
> > 4. If arch=rv64i3p0, then ".option arch, +i" will update the i's version
> >    from 2.0 to the default one according to the chosen isa spec.
> >
> > bfd/
> >       * elfxx-riscv.c (riscv_add_subset): If the subset is already added,
> >       and the new versions are not RISCV_UNKNOWN_VERSION, then update the
> >       versions to the subset list.
> >       (riscv_copy_subset): New function.  Copy the subset from list.
> >       (riscv_copy_subset_list): New function.  Return the new copyed
> list.
> >       (riscv_update_subset): Updated to make .option arch directives
> workable.
> >       * elfxx-riscv.h: Updated.
> > gas/
> >       * config/tc-riscv.c (riscv_subsets): Defined as a pointer.
> >       (riscv_rps_as): Init the subset_list to NULL, we will set it later
> >       once riscv_opts_stack is created or updated.
> >       (struct riscv_option_stack, riscv_opts_stack): Moved forward.
> >       (riscv_set_arch): Updated.
> >       (s_riscv_option): Support new .option arch directive, to add,
> remove
> >       or update subsets for the specific code region.
> >       (riscv_write_out_attrs): Updated.
> >       * doc/c-riscv.texi: Added document for new .option arch directive.
> >       * testsuite/gas/riscv/option-arch-01a.d: New testcase.
> >       * testsuite/gas/riscv/option-arch-01b.d: Likewise.
> >       * testsuite/gas/riscv/option-arch-01.s: Likewise..
> >       * testsuite/gas/riscv/option-arch-02.d: Likewise.
> >       * testsuite/gas/riscv/option-arch-02.s: Likewise.
> >       * testsuite/gas/riscv/option-arch-fail.d: Likewise.
> >       * testsuite/gas/riscv/option-arch-fail.l: Likewise.
> >       * testsuite/gas/riscv/option-arch-fail.s: Likewise.
> > ---
> >  bfd/elfxx-riscv.c                          | 158
> ++++++++++++++++++++++++-----
> >  bfd/elfxx-riscv.h                          |   5 +-
> >  gas/config/tc-riscv.c                      |  57 +++++++----
> >  gas/doc/c-riscv.texi                       |  15 ++-
> >  gas/testsuite/gas/riscv/option-arch-01.s   |  10 ++
> >  gas/testsuite/gas/riscv/option-arch-01a.d  |  14 +++
> >  gas/testsuite/gas/riscv/option-arch-01b.d  |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-02.d   |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-02.s   |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-03.d   |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-03.s   |   3 +
> >  gas/testsuite/gas/riscv/option-arch-fail.d |   3 +
> >  gas/testsuite/gas/riscv/option-arch-fail.l |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-fail.s |  10 ++
> >  14 files changed, 272 insertions(+), 43 deletions(-)
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-01.s
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-01a.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-01b.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-02.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-02.s
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-03.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-03.s
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.l
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.s
> >
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index 91afd4c..b8da40c 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1468,7 +1468,15 @@ riscv_add_subset (riscv_subset_list_t
> *subset_list,
> >    riscv_subset_t *current, *new;
> >
> >    if (riscv_lookup_subset (subset_list, subset, &current))
> > -    return;
> > +    {
> > +      if (major != RISCV_UNKNOWN_VERSION
> > +       && minor != RISCV_UNKNOWN_VERSION)
> > +     {
> > +       current->major_version = major;
> > +       current->minor_version = minor;
> > +     }
> > +      return;
> > +    }
> >
> >    new = xmalloc (sizeof *new);
> >    new->name = xstrdup (subset);
> > @@ -2138,6 +2146,37 @@ riscv_arch_str (unsigned xlen, const
> riscv_subset_list_t *subset)
> >    return attr_str;
> >  }
> >
> > +/* Copy the subset in the subset list.  */
> > +
> > +static struct riscv_subset_t *
> > +riscv_copy_subset (riscv_subset_list_t *subset_list,
> > +                riscv_subset_t *subset)
> > +{
> > +  if (subset == NULL)
> > +    return NULL;
> > +
> > +  riscv_subset_t *new = xmalloc (sizeof *new);
> > +  new->name = xstrdup (subset->name);
> > +  new->major_version = subset->major_version;
> > +  new->minor_version = subset->minor_version;
> > +  new->next = riscv_copy_subset (subset_list, subset->next);
> > +
> > +  if (subset->next == NULL)
> > +    subset_list->tail = new;
> > +
> > +  return new;
> > +}
> > +
> > +/* Copy the subset list.  */
> > +
> > +riscv_subset_list_t *
> > +riscv_copy_subset_list (riscv_subset_list_t *subset_list)
> > +{
> > +  riscv_subset_list_t *new = xmalloc (sizeof *new);
> > +  new->head = riscv_copy_subset (new, subset_list->head);
> > +  return new;
> > +}
> > +
> >  /* Remove the SUBSET from the subset list.  */
> >
> >  static void
> > @@ -2164,40 +2203,111 @@ riscv_remove_subset (riscv_subset_list_t
> *subset_list,
> >  }
> >
> >  /* Add/Remove an extension to/from the subset list.  This is used for
> > -   the .option rvc or norvc.  */
> > +   the .option rvc or norvc, and .option arch directives.  */
> >
> >  bool
> >  riscv_update_subset (riscv_parse_subset_t *rps,
> > -                  const char *subset,
> > -                  bool removed)
> > +                  const char *str)
> >  {
> > -  if (strlen (subset) == 0
> > -      || (strlen (subset) == 1
> > -       && riscv_ext_order[(*subset - 'a')] == 0)
> > -      || (strlen (subset) > 1
> > -       && rps->check_unknown_prefixed_ext
> > -       && !riscv_recognized_prefixed_ext (subset)))
> > -    {
> > -      rps->error_handler
> > -     (_("riscv_update_subset: unknown ISA extension `%s'"), subset);
> > -      return false;
> > -    }
> > +  const char *p = str;
> >
> > -  if (removed)
> > +  do
> >      {
> > -      if (strcmp (subset, "i") == 0)
> > +      int major_version = RISCV_UNKNOWN_VERSION;
> > +      int minor_version = RISCV_UNKNOWN_VERSION;
> > +
> > +      bool removed = false;
> > +      switch (*p++)
> > +     {
> > +     case '+': removed = false; break;
> > +     case '-': removed = true; break;
> > +     case '=':
> > +       riscv_release_subset_list (rps->subset_list);
> > +       return riscv_parse_subset (rps, p);
> > +     default:
> > +       rps->error_handler
> > +         (_("extensions must begin with +/-/= in .option arch `%s'"),
> str);
> > +       return false;
> > +     }
> > +
> > +      char *subset = xstrdup (p);
> > +      char *q = subset;
> > +      const char *end_of_version;
> > +      /* Extract the whole prefixed extension by ','.  */
> > +      while (*q != '\0' && *q != ',')
> > +        q++;
> > +      /* Look forward to the first letter which is not
> <major>p<minor>.  */
> > +      bool find_any_version = false;
> > +      bool find_minor_version = false;
> > +      while (1)
> > +        {
> > +       q--;
> > +       if (ISDIGIT (*q))
> > +         find_any_version = true;
> > +       else if (find_any_version
> > +                && !find_minor_version
> > +                && *q == 'p'
> > +                && ISDIGIT (*(q - 1)))
> > +         find_minor_version = true;
> > +       else
> > +         break;
> > +     }
> > +      q++;
> > +      /* Check if the end of extension is 'p' or not.  If yes, then
> > +      the second letter from the end cannot be number.  */
> > +      if (*(q - 1) == 'p' && ISDIGIT (*(q - 2)))
> > +     {
> > +       *q = '\0';
> > +       rps->error_handler
> > +         (_("invalid ISA extension ends with <number>p "
> > +            "in .option arch `%s'"), str);
> > +       free (subset);
> > +       return false;
> > +     }
> > +      end_of_version =
> > +     riscv_parsing_subset_version (q, &major_version, &minor_version);
> > +      *q = '\0';
> > +      if (end_of_version == NULL)
> > +     {
> > +       free (subset);
> > +       return false;
> > +     }
> > +
> > +      if (strlen (subset) == 0
> > +       || (strlen (subset) == 1
> > +           && riscv_ext_order[(*subset - 'a')] == 0)
> > +       || (strlen (subset) > 1
> > +           && rps->check_unknown_prefixed_ext
> > +           && !riscv_recognized_prefixed_ext (subset)))
> >       {
> >         rps->error_handler
> > -         (_("riscv_update_subset: cannot remove extension i from "
> > -            "the subset list"));
> > +         (_("unknown ISA extension `%s' in .option arch `%s'"),
> > +          subset, str);
> > +       free (subset);
> >         return false;
> >       }
> > -      riscv_remove_subset (rps->subset_list, subset);
> > +
> > +      if (removed)
> > +     {
> > +       if (strcmp (subset, "i") == 0)
> > +         {
> > +           rps->error_handler
> > +             (_("cannot remove extension `i' in .option arch `%s'"),
> str);
> > +           free (subset);
> > +           return false;
> > +         }
> > +       riscv_remove_subset (rps->subset_list, subset);
> > +     }
> > +      else
> > +     riscv_parse_add_subset (rps, subset, major_version, minor_version,
> true);
> > +      p += end_of_version - subset;
> > +      free (subset);
> >      }
> > -  else
> > -    riscv_parse_add_subset (rps, subset,
> > -                         RISCV_UNKNOWN_VERSION,
> > -                         RISCV_UNKNOWN_VERSION, true);
> > +  while (*p++ == ',');
> > +
> > +  if (*(--p) != '\0')
> > +    rps->error_handler
> > +      (_("unexpected value in .option arch `%s'"), str);
> >
> >    riscv_parse_add_implicit_subsets (rps);
> >    return riscv_parse_check_conflicts (rps);
> > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> > index 8de9adc..ea7edc4 100644
> > --- a/bfd/elfxx-riscv.h
> > +++ b/bfd/elfxx-riscv.h
> > @@ -92,8 +92,11 @@ riscv_estimate_digit (unsigned);
> >  extern int
> >  riscv_compare_subsets (const char *, const char *);
> >
> > +extern riscv_subset_list_t *
> > +riscv_copy_subset_list (riscv_subset_list_t *);
> > +
> >  extern bool
> > -riscv_update_subset (riscv_parse_subset_t *, const char *, bool);
> > +riscv_update_subset (riscv_parse_subset_t *, const char *);
> >
> >  extern bool
> >  riscv_subset_supports (riscv_parse_subset_t *, const char *);
> > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> > index 930cfb2..da3d911 100644
> > --- a/gas/config/tc-riscv.c
> > +++ b/gas/config/tc-riscv.c
> > @@ -235,16 +235,27 @@ riscv_set_rvc (bool rvc_value)
> >     the architecture string.  The architecture string can be set by the
> >     -march option, the elf architecture attributes, and the --with-arch
> >     configure option.  */
> > -static riscv_subset_list_t riscv_subsets;
> > +static riscv_subset_list_t *riscv_subsets = NULL;
> >  static riscv_parse_subset_t riscv_rps_as =
> >  {
> > -  &riscv_subsets,    /* subset_list.  */
> > +  NULL,                      /* subset_list, we will set it later once
> > +                        riscv_opts_stack is created or updated.  */
> >    as_bad,            /* error_handler.  */
> >    &xlen,             /* xlen.  */
> >    &default_isa_spec, /* isa_spec.  */
> >    true,                      /* check_unknown_prefixed_ext.  */
> >  };
> >
> > +/* This structure is used to hold a stack of .option values.  */
> > +struct riscv_option_stack
> > +{
> > +  struct riscv_option_stack *next;
> > +  struct riscv_set_options options;
> > +  riscv_subset_list_t *subset_list;
> > +};
> > +
> > +static struct riscv_option_stack *riscv_opts_stack = NULL;
> > +
> >  /* Set which ISA and extensions are available.  */
> >
> >  static void
> > @@ -257,7 +268,14 @@ riscv_set_arch (const char *s)
> >        return;
> >      }
> >
> > -  riscv_release_subset_list (&riscv_subsets);
> > +  if (riscv_subsets == NULL)
> > +    {
> > +      riscv_subsets = XNEW (riscv_subset_list_t);
> > +      riscv_subsets->head = NULL;
> > +      riscv_subsets->tail = NULL;
> > +      riscv_rps_as.subset_list = riscv_subsets;
> > +    }
> > +  riscv_release_subset_list (riscv_subsets);
> >    riscv_parse_subset (&riscv_rps_as, s);
> >
> >    riscv_set_rvc (false);
> > @@ -3715,15 +3733,6 @@ riscv_pre_output_hook (void)
> >    subseg_set (seg, subseg);
> >  }
> >
> > -/* This structure is used to hold a stack of .option values.  */
> > -struct riscv_option_stack
> > -{
> > -  struct riscv_option_stack *next;
> > -  struct riscv_set_options options;
> > -};
> > -
> > -static struct riscv_option_stack *riscv_opts_stack;
> > -
> >  /* Handle the .option pseudo-op.  */
> >
> >  static void
> > @@ -3738,12 +3747,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
> >
> >    if (strcmp (name, "rvc") == 0)
> >      {
> > -      riscv_update_subset (&riscv_rps_as, "c", false);
> > +      riscv_update_subset (&riscv_rps_as, "+c");
> >        riscv_set_rvc (true);
> >      }
> >    else if (strcmp (name, "norvc") == 0)
> >      {
> > -      riscv_update_subset (&riscv_rps_as, "c", true);
> > +      riscv_update_subset (&riscv_rps_as, "-c");
> >        riscv_set_rvc (false);
> >      }
> >    else if (strcmp (name, "pic") == 0)
> > @@ -3758,14 +3767,24 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
> >      riscv_opts.csr_check = true;
> >    else if (strcmp (name, "no-csr-check") == 0)
> >      riscv_opts.csr_check = false;
> > +  else if (strncmp (name, "arch,", 5) == 0)
> > +    {
> > +      name += 5;
> > +      if (ISSPACE (*name) && *name != '\0')
> > +     name++;
> > +      riscv_update_subset (&riscv_rps_as, name);
> > +    }
> >    else if (strcmp (name, "push") == 0)
> >      {
> >        struct riscv_option_stack *s;
> >
> > -      s = (struct riscv_option_stack *) xmalloc (sizeof *s);
> > +      s = XNEW (struct riscv_option_stack);
> >        s->next = riscv_opts_stack;
> >        s->options = riscv_opts;
> > +      s->subset_list = riscv_subsets;
> >        riscv_opts_stack = s;
> > +      riscv_subsets = riscv_copy_subset_list (s->subset_list);
> > +      riscv_rps_as.subset_list = riscv_subsets;
> >      }
> >    else if (strcmp (name, "pop") == 0)
> >      {
> > @@ -3776,8 +3795,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
> >       as_bad (_(".option pop with no .option push"));
> >        else
> >       {
> > -       riscv_opts = s->options;
> > +       riscv_subset_list_t *release_subsets = riscv_subsets;
> >         riscv_opts_stack = s->next;
> > +       riscv_opts = s->options;
> > +       riscv_subsets = s->subset_list;
> > +       riscv_rps_as.subset_list = riscv_subsets;
> > +       riscv_release_subset_list (release_subsets);
> >         free (s);
> >       }
> >      }
> > @@ -4262,7 +4285,7 @@ riscv_write_out_attrs (void)
> >    unsigned int i;
> >
> >    /* Re-write architecture elf attribute.  */
> > -  arch_str = riscv_arch_str (xlen, &riscv_subsets);
> > +  arch_str = riscv_arch_str (xlen, riscv_subsets);
> >    bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str);
> >    xfree ((void *) arch_str);
> >
> > diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
> > index bfbf61d..697be3a 100644
> > --- a/gas/doc/c-riscv.texi
> > +++ b/gas/doc/c-riscv.texi
> > @@ -194,7 +194,7 @@ command-line options are respected for the bulk of
> the file being assembled.
> >  @itemx norvc
> >  Enables or disables the generation of compressed instructions.
> Instructions
> >  are opportunistically compressed by the RISC-V assembler when possible,
> but
> > -sometimes this behavior is not desirable.
> > +sometimes this behavior is not desirable, especially when handling
> alignments.
> >
> >  @item pic
> >  @itemx nopic
> > @@ -212,6 +212,19 @@ desirable.
> >  @itemx no-csr-check
> >  Enables or disables the CSR checking.
> >
> > +@item arch, @var{+extension[version]}
> [,...,@var{+extension_n[version_n]}]
> > +@itemx arch, @var{-extension} [,...,@var{-extension_n}]
> > +@itemx arch, @var{=ISA}
> > +Enables or disables the extensions for specific code region.  For
> example,
> > +@samp{.option arch, +m2p0} means add m extension with version 2.0, and
> > +@samp{.option arch, -f, -d} means remove extensions, f and d, from the
> > +architecture string.  Note that, @samp{.option arch, +c, -c} have the
> same
> > +behavior as @samp{.option rvc, norvc}.  However, they are also
> undesirable
> > +sometimes.  Besides, @samp{.option arch, -i} is illegal, since we cannot
> > +remove the base i extension anytime.  If you want to reset the whole ISA
> > +string, you can also use @samp{.option arch, =rv32imac} to overwrite the
> > +previous settings.
> > +
> >  @cindex INSN directives
> >  @item .insn @var{type}, @var{operand} [,...,@var{operand_n}]
> >  @itemx .insn @var{insn_length}, @var{value}
> > diff --git a/gas/testsuite/gas/riscv/option-arch-01.s
> b/gas/testsuite/gas/riscv/option-arch-01.s
> > new file mode 100644
> > index 0000000..201f9b3
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-01.s
> > @@ -0,0 +1,10 @@
> > +.attribute arch, "rv64ic"
> > +add  a0, a0, a1
> > +.option push
> > +.option arch, +d2p0, -c, +xvendor1p0
> > +add  a0, a0, a1
> > +frcsr        a0      # Should add mapping symbol with ISA here, and
> then dump it to frcsr.
> > +.option push
> > +.option arch, +i3p0, +m3p0, +d3p0
> > +.option pop
> > +.option pop
> > diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d
> b/gas/testsuite/gas/riscv/option-arch-01a.d
> > new file mode 100644
> > index 0000000..59bc1d2
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-01a.d
> > @@ -0,0 +1,14 @@
> > +#as:
> > +#source: option-arch-01.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[    ]+[0-9a-f]+:[   ]+952e[         ]+add[          ]+a0,a0,a1
> > +[    ]+[0-9a-f]+:[   ]+00b50533[     ]+add[          ]+a0,a0,a1
> > +[    ]+[0-9a-f]+:[   ]+00302573[     ]+csrr[         ]+a0,fcsr
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-01b.d
> b/gas/testsuite/gas/riscv/option-arch-01b.d
> > new file mode 100644
> > index 0000000..9a6c2c5
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-01b.d
> > @@ -0,0 +1,8 @@
> > +#as:
> > +#readelf: -A
> > +#source: option-arch-01.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv64i2p0_c2p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-02.d
> b/gas/testsuite/gas/riscv/option-arch-02.d
> > new file mode 100644
> > index 0000000..0fe89ec
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-02.d
> > @@ -0,0 +1,8 @@
> > +#as:
> > +#readelf: -A
> > +#source: option-arch-02.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv64i3p0_m3p0_f2p0_d3p0_c2p0_xvendor32x3p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-02.s
> b/gas/testsuite/gas/riscv/option-arch-02.s
> > new file mode 100644
> > index 0000000..f4ceee84
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-02.s
> > @@ -0,0 +1,8 @@
> > +.attribute arch, "rv64ic"
> > +add  a0, a0, a1
> > +.option push
> > +.option arch, +d2p0, -c, +xvendor1p0
> > +add  a0, a0, a1
> > +frcsr        a0
> > +.option pop
> > +.option arch, +i3p0, +m3p0, +d3p0, +xvendor32x3p0
> > diff --git a/gas/testsuite/gas/riscv/option-arch-03.d
> b/gas/testsuite/gas/riscv/option-arch-03.d
> > new file mode 100644
> > index 0000000..b621d03
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-03.d
> > @@ -0,0 +1,8 @@
> > +#as:
> > +#readelf: -A
> > +#source: option-arch-03.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv32i2p0_c2p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-03.s
> b/gas/testsuite/gas/riscv/option-arch-03.s
> > new file mode 100644
> > index 0000000..7183140
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-03.s
> > @@ -0,0 +1,3 @@
> > +.attribute arch, "rv64ic"
> > +.option arch, +d2p0, -c
> > +.option arch, =rv32ic
> > diff --git a/gas/testsuite/gas/riscv/option-arch-fail.d
> b/gas/testsuite/gas/riscv/option-arch-fail.d
> > new file mode 100644
> > index 0000000..bce5d32
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-fail.d
> > @@ -0,0 +1,3 @@
> > +#as:
> > +#source: option-arch-fail.s
> > +#error_output: option-arch-fail.l
> > diff --git a/gas/testsuite/gas/riscv/option-arch-fail.l
> b/gas/testsuite/gas/riscv/option-arch-fail.l
> > new file mode 100644
> > index 0000000..3e0599e
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-fail.l
> > @@ -0,0 +1,8 @@
> > +.*Assembler messages:
> > +.*Error: extensions must begin with \+/\-/\= in .option arch `m2p0'
> > +.*Error: cannot remove extension `i' in .option arch `\-i'
> > +.*Error: unknown ISA extension `zsubset' in .option arch `\+zsubset2p0'
> > +.*Error: unknown ISA extension `f2p0_d' in .option arch `\+f2p0_d2p0'
> > +.*Error: unknown ISA extension `' in .option arch `\+'
> > +.*Error: invalid ISA extension ends with <number>p in .option arch
> `\+xvendor2p'
> > +.*Error: .option pop with no .option push
> > diff --git a/gas/testsuite/gas/riscv/option-arch-fail.s
> b/gas/testsuite/gas/riscv/option-arch-fail.s
> > new file mode 100644
> > index 0000000..a0b1bde
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-fail.s
> > @@ -0,0 +1,10 @@
> > +.attribute arch, "rv64ic"
> > +.option push
> > +.option arch, m2p0
> > +.option arch, -i
> > +.option arch, +zsubset2p0
> > +.option arch, +f2p0_d2p0
> > +.option arch, +
> > +.option arch, +xvendor2p
> > +.option pop
> > +.option pop
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [committed v3] RISC-V: Support new .option arch directive.
  2021-11-20  2:17   ` Nelson Chu
@ 2021-11-20  2:32     ` Jessica Clarke
  0 siblings, 0 replies; 4+ messages in thread
From: Jessica Clarke @ 2021-11-20  2:32 UTC (permalink / raw)
  To: Nelson Chu; +Cc: Palmer Dabbelt, Jim Wilson, binutils, Kito Cheng

On 20 Nov 2021, at 02:17, Nelson Chu <nelson.chu@sifive.com> wrote:
> 
> 
> On Fri, 19 Nov 2021 02:53:53 PST (-0800), Nelson Chu wrote:
> > https://github.com/riscv/riscv-asm-manual/pull/67
> >
> > Format:
> > .option arch, +<extension><version>, ...
> > .option arch, -<extension>
> > .option arch, =<ISA string>
> >
> > The new direcitve is used to enable/disable extensions for the specific
> > code region.  For example,
> >
> > .attribute arch, "rv64ic"   # arch = rv64i2p0_c2p0
> > .option push
> > .option arch, +d2p0, -c     # arch = rv64i2p0_f2p0_d2p0, f is added implied
> > .option arch, =rv32gc       # arch = rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0

This doesn’t make sense, or is at least inconsistent with -march.
.option arch, =foo is effectively arch = =foo, the = is already
implicit. For -march we have -march=foo, and -march=[+-]foo has been
suggested, so to be consistent with that it should just be .option
arch, foo.

> > .option pop                 # arch = rv64i2p0_c2p0
> >
> > Note that,
> > 1. ".option rvc/norvc" have the same behavior as ".option arch +c/-c".
> > 2. ".option arch -i" is illegal, since we cannot remove base i extension.
> 
> Should that apply to E as well?  RV32E is as much of a base ISA as RV32I 
> is, at least according to the specs, but this allows something like

RV32E isn’t ratified, and only implemented in GCC/binutils. I can see
it being useful to do +/-e, though I’d write it as +/-rv32e, because
*that* is the base ISA. But, whilst .option arch, -rv32i, +rv32i makes
sense as a thing to write, .option arch, -rv32i does not. So I would
say ban it for now, and if it turns out to be necessary then people can
add it later, though I suspect it won’t be; mixing RV32E and RV32I
seems pretty niche, I can see mixing RV32I and RV64I as being more
common, though suspect even then that’s not needed since it’s not
legacy x86 where you have 16-bit, 32-bit and 64-bit code all
co-existing to bootstrap yourself out of the legacy architectures.

>    .attribute arch, "rv32e"
>    .option arch, -e
>    add x31, x31, x31
> 
> to turn E into I.
> 
> It does feel kind of nice to treat E as an extension here, as that would 
> allow us to mix in short E-compatible code segments (to try and detect 
> the base ISA, for example), but the ISA doesn't treat it that way so 
> it's kind of odd to do so in the toolchain.
> 
> I'm not really sure I have a case where "-e" is any better than 
> "=rv32e", though, so maybe it's better to just play it save and reject 
> +/-e?
> 
> This make sense to me, +- base extension looks weird, and we probably also need to reject G extension when using both the operation +-.  Instead, if users want to add or update the version of base extensions, which are I, E and G, then they should use the operation = and reset the whole architecture string.
> 
> Hi Kito, Hi Jessica,
> 
> Does this also make sense to you?  It would be great if this work for you and could update to the psabi pr. Or maybe you have a better idea :)
> 
> Thank you very much
> Nelson
> 
> 
> > 3. If arch=rv64i2p0, then ".option arch, +i3p0" will update the i's version
> >    from 2.0 to 3.0.
> > 4. If arch=rv64i3p0, then ".option arch, +i" will update the i's version
> >    from 2.0 to the default one according to the chosen isa spec.

I think you mean 3.0 here not 2.0. But this seems dodgy; what if the
current arch string has a newer version? Then you’re going backwards.
But also, going forwards isn’t guaranteed to be backwards-compatible;
going from 2.0 to 2.1 splits out Zicsr, so +i3p0 *removes*
instructions. This needs more thought.

Jess

> > bfd/
> >       * elfxx-riscv.c (riscv_add_subset): If the subset is already added,
> >       and the new versions are not RISCV_UNKNOWN_VERSION, then update the
> >       versions to the subset list.
> >       (riscv_copy_subset): New function.  Copy the subset from list.
> >       (riscv_copy_subset_list): New function.  Return the new copyed list.
> >       (riscv_update_subset): Updated to make .option arch directives workable.
> >       * elfxx-riscv.h: Updated.
> > gas/
> >       * config/tc-riscv.c (riscv_subsets): Defined as a pointer.
> >       (riscv_rps_as): Init the subset_list to NULL, we will set it later
> >       once riscv_opts_stack is created or updated.
> >       (struct riscv_option_stack, riscv_opts_stack): Moved forward.
> >       (riscv_set_arch): Updated.
> >       (s_riscv_option): Support new .option arch directive, to add, remove
> >       or update subsets for the specific code region.
> >       (riscv_write_out_attrs): Updated.
> >       * doc/c-riscv.texi: Added document for new .option arch directive.
> >       * testsuite/gas/riscv/option-arch-01a.d: New testcase.
> >       * testsuite/gas/riscv/option-arch-01b.d: Likewise.
> >       * testsuite/gas/riscv/option-arch-01.s: Likewise..
> >       * testsuite/gas/riscv/option-arch-02.d: Likewise.
> >       * testsuite/gas/riscv/option-arch-02.s: Likewise.
> >       * testsuite/gas/riscv/option-arch-fail.d: Likewise.
> >       * testsuite/gas/riscv/option-arch-fail.l: Likewise.
> >       * testsuite/gas/riscv/option-arch-fail.s: Likewise.
> > ---
> >  bfd/elfxx-riscv.c                          | 158 ++++++++++++++++++++++++-----
> >  bfd/elfxx-riscv.h                          |   5 +-
> >  gas/config/tc-riscv.c                      |  57 +++++++----
> >  gas/doc/c-riscv.texi                       |  15 ++-
> >  gas/testsuite/gas/riscv/option-arch-01.s   |  10 ++
> >  gas/testsuite/gas/riscv/option-arch-01a.d  |  14 +++
> >  gas/testsuite/gas/riscv/option-arch-01b.d  |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-02.d   |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-02.s   |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-03.d   |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-03.s   |   3 +
> >  gas/testsuite/gas/riscv/option-arch-fail.d |   3 +
> >  gas/testsuite/gas/riscv/option-arch-fail.l |   8 ++
> >  gas/testsuite/gas/riscv/option-arch-fail.s |  10 ++
> >  14 files changed, 272 insertions(+), 43 deletions(-)
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-01.s
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-01a.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-01b.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-02.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-02.s
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-03.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-03.s
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.d
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.l
> >  create mode 100644 gas/testsuite/gas/riscv/option-arch-fail.s
> >
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index 91afd4c..b8da40c 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1468,7 +1468,15 @@ riscv_add_subset (riscv_subset_list_t *subset_list,
> >    riscv_subset_t *current, *new;
> >
> >    if (riscv_lookup_subset (subset_list, subset, &current))
> > -    return;
> > +    {
> > +      if (major != RISCV_UNKNOWN_VERSION
> > +       && minor != RISCV_UNKNOWN_VERSION)
> > +     {
> > +       current->major_version = major;
> > +       current->minor_version = minor;
> > +     }
> > +      return;
> > +    }
> >
> >    new = xmalloc (sizeof *new);
> >    new->name = xstrdup (subset);
> > @@ -2138,6 +2146,37 @@ riscv_arch_str (unsigned xlen, const riscv_subset_list_t *subset)
> >    return attr_str;
> >  }
> >
> > +/* Copy the subset in the subset list.  */
> > +
> > +static struct riscv_subset_t *
> > +riscv_copy_subset (riscv_subset_list_t *subset_list,
> > +                riscv_subset_t *subset)
> > +{
> > +  if (subset == NULL)
> > +    return NULL;
> > +
> > +  riscv_subset_t *new = xmalloc (sizeof *new);
> > +  new->name = xstrdup (subset->name);
> > +  new->major_version = subset->major_version;
> > +  new->minor_version = subset->minor_version;
> > +  new->next = riscv_copy_subset (subset_list, subset->next);
> > +
> > +  if (subset->next == NULL)
> > +    subset_list->tail = new;
> > +
> > +  return new;
> > +}
> > +
> > +/* Copy the subset list.  */
> > +
> > +riscv_subset_list_t *
> > +riscv_copy_subset_list (riscv_subset_list_t *subset_list)
> > +{
> > +  riscv_subset_list_t *new = xmalloc (sizeof *new);
> > +  new->head = riscv_copy_subset (new, subset_list->head);
> > +  return new;
> > +}
> > +
> >  /* Remove the SUBSET from the subset list.  */
> >
> >  static void
> > @@ -2164,40 +2203,111 @@ riscv_remove_subset (riscv_subset_list_t *subset_list,
> >  }
> >
> >  /* Add/Remove an extension to/from the subset list.  This is used for
> > -   the .option rvc or norvc.  */
> > +   the .option rvc or norvc, and .option arch directives.  */
> >
> >  bool
> >  riscv_update_subset (riscv_parse_subset_t *rps,
> > -                  const char *subset,
> > -                  bool removed)
> > +                  const char *str)
> >  {
> > -  if (strlen (subset) == 0
> > -      || (strlen (subset) == 1
> > -       && riscv_ext_order[(*subset - 'a')] == 0)
> > -      || (strlen (subset) > 1
> > -       && rps->check_unknown_prefixed_ext
> > -       && !riscv_recognized_prefixed_ext (subset)))
> > -    {
> > -      rps->error_handler
> > -     (_("riscv_update_subset: unknown ISA extension `%s'"), subset);
> > -      return false;
> > -    }
> > +  const char *p = str;
> >
> > -  if (removed)
> > +  do
> >      {
> > -      if (strcmp (subset, "i") == 0)
> > +      int major_version = RISCV_UNKNOWN_VERSION;
> > +      int minor_version = RISCV_UNKNOWN_VERSION;
> > +
> > +      bool removed = false;
> > +      switch (*p++)
> > +     {
> > +     case '+': removed = false; break;
> > +     case '-': removed = true; break;
> > +     case '=':
> > +       riscv_release_subset_list (rps->subset_list);
> > +       return riscv_parse_subset (rps, p);
> > +     default:
> > +       rps->error_handler
> > +         (_("extensions must begin with +/-/= in .option arch `%s'"), str);
> > +       return false;
> > +     }
> > +
> > +      char *subset = xstrdup (p);
> > +      char *q = subset;
> > +      const char *end_of_version;
> > +      /* Extract the whole prefixed extension by ','.  */
> > +      while (*q != '\0' && *q != ',')
> > +        q++;
> > +      /* Look forward to the first letter which is not <major>p<minor>.  */
> > +      bool find_any_version = false;
> > +      bool find_minor_version = false;
> > +      while (1)
> > +        {
> > +       q--;
> > +       if (ISDIGIT (*q))
> > +         find_any_version = true;
> > +       else if (find_any_version
> > +                && !find_minor_version
> > +                && *q == 'p'
> > +                && ISDIGIT (*(q - 1)))
> > +         find_minor_version = true;
> > +       else
> > +         break;
> > +     }
> > +      q++;
> > +      /* Check if the end of extension is 'p' or not.  If yes, then
> > +      the second letter from the end cannot be number.  */
> > +      if (*(q - 1) == 'p' && ISDIGIT (*(q - 2)))
> > +     {
> > +       *q = '\0';
> > +       rps->error_handler
> > +         (_("invalid ISA extension ends with <number>p "
> > +            "in .option arch `%s'"), str);
> > +       free (subset);
> > +       return false;
> > +     }
> > +      end_of_version =
> > +     riscv_parsing_subset_version (q, &major_version, &minor_version);
> > +      *q = '\0';
> > +      if (end_of_version == NULL)
> > +     {
> > +       free (subset);
> > +       return false;
> > +     }
> > +
> > +      if (strlen (subset) == 0
> > +       || (strlen (subset) == 1
> > +           && riscv_ext_order[(*subset - 'a')] == 0)
> > +       || (strlen (subset) > 1
> > +           && rps->check_unknown_prefixed_ext
> > +           && !riscv_recognized_prefixed_ext (subset)))
> >       {
> >         rps->error_handler
> > -         (_("riscv_update_subset: cannot remove extension i from "
> > -            "the subset list"));
> > +         (_("unknown ISA extension `%s' in .option arch `%s'"),
> > +          subset, str);
> > +       free (subset);
> >         return false;
> >       }
> > -      riscv_remove_subset (rps->subset_list, subset);
> > +
> > +      if (removed)
> > +     {
> > +       if (strcmp (subset, "i") == 0)
> > +         {
> > +           rps->error_handler
> > +             (_("cannot remove extension `i' in .option arch `%s'"), str);
> > +           free (subset);
> > +           return false;
> > +         }
> > +       riscv_remove_subset (rps->subset_list, subset);
> > +     }
> > +      else
> > +     riscv_parse_add_subset (rps, subset, major_version, minor_version, true);
> > +      p += end_of_version - subset;
> > +      free (subset);
> >      }
> > -  else
> > -    riscv_parse_add_subset (rps, subset,
> > -                         RISCV_UNKNOWN_VERSION,
> > -                         RISCV_UNKNOWN_VERSION, true);
> > +  while (*p++ == ',');
> > +
> > +  if (*(--p) != '\0')
> > +    rps->error_handler
> > +      (_("unexpected value in .option arch `%s'"), str);
> >
> >    riscv_parse_add_implicit_subsets (rps);
> >    return riscv_parse_check_conflicts (rps);
> > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> > index 8de9adc..ea7edc4 100644
> > --- a/bfd/elfxx-riscv.h
> > +++ b/bfd/elfxx-riscv.h
> > @@ -92,8 +92,11 @@ riscv_estimate_digit (unsigned);
> >  extern int
> >  riscv_compare_subsets (const char *, const char *);
> >
> > +extern riscv_subset_list_t *
> > +riscv_copy_subset_list (riscv_subset_list_t *);
> > +
> >  extern bool
> > -riscv_update_subset (riscv_parse_subset_t *, const char *, bool);
> > +riscv_update_subset (riscv_parse_subset_t *, const char *);
> >
> >  extern bool
> >  riscv_subset_supports (riscv_parse_subset_t *, const char *);
> > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> > index 930cfb2..da3d911 100644
> > --- a/gas/config/tc-riscv.c
> > +++ b/gas/config/tc-riscv.c
> > @@ -235,16 +235,27 @@ riscv_set_rvc (bool rvc_value)
> >     the architecture string.  The architecture string can be set by the
> >     -march option, the elf architecture attributes, and the --with-arch
> >     configure option.  */
> > -static riscv_subset_list_t riscv_subsets;
> > +static riscv_subset_list_t *riscv_subsets = NULL;
> >  static riscv_parse_subset_t riscv_rps_as =
> >  {
> > -  &riscv_subsets,    /* subset_list.  */
> > +  NULL,                      /* subset_list, we will set it later once
> > +                        riscv_opts_stack is created or updated.  */
> >    as_bad,            /* error_handler.  */
> >    &xlen,             /* xlen.  */
> >    &default_isa_spec, /* isa_spec.  */
> >    true,                      /* check_unknown_prefixed_ext.  */
> >  };
> >
> > +/* This structure is used to hold a stack of .option values.  */
> > +struct riscv_option_stack
> > +{
> > +  struct riscv_option_stack *next;
> > +  struct riscv_set_options options;
> > +  riscv_subset_list_t *subset_list;
> > +};
> > +
> > +static struct riscv_option_stack *riscv_opts_stack = NULL;
> > +
> >  /* Set which ISA and extensions are available.  */
> >
> >  static void
> > @@ -257,7 +268,14 @@ riscv_set_arch (const char *s)
> >        return;
> >      }
> >
> > -  riscv_release_subset_list (&riscv_subsets);
> > +  if (riscv_subsets == NULL)
> > +    {
> > +      riscv_subsets = XNEW (riscv_subset_list_t);
> > +      riscv_subsets->head = NULL;
> > +      riscv_subsets->tail = NULL;
> > +      riscv_rps_as.subset_list = riscv_subsets;
> > +    }
> > +  riscv_release_subset_list (riscv_subsets);
> >    riscv_parse_subset (&riscv_rps_as, s);
> >
> >    riscv_set_rvc (false);
> > @@ -3715,15 +3733,6 @@ riscv_pre_output_hook (void)
> >    subseg_set (seg, subseg);
> >  }
> >
> > -/* This structure is used to hold a stack of .option values.  */
> > -struct riscv_option_stack
> > -{
> > -  struct riscv_option_stack *next;
> > -  struct riscv_set_options options;
> > -};
> > -
> > -static struct riscv_option_stack *riscv_opts_stack;
> > -
> >  /* Handle the .option pseudo-op.  */
> >
> >  static void
> > @@ -3738,12 +3747,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
> >
> >    if (strcmp (name, "rvc") == 0)
> >      {
> > -      riscv_update_subset (&riscv_rps_as, "c", false);
> > +      riscv_update_subset (&riscv_rps_as, "+c");
> >        riscv_set_rvc (true);
> >      }
> >    else if (strcmp (name, "norvc") == 0)
> >      {
> > -      riscv_update_subset (&riscv_rps_as, "c", true);
> > +      riscv_update_subset (&riscv_rps_as, "-c");
> >        riscv_set_rvc (false);
> >      }
> >    else if (strcmp (name, "pic") == 0)
> > @@ -3758,14 +3767,24 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
> >      riscv_opts.csr_check = true;
> >    else if (strcmp (name, "no-csr-check") == 0)
> >      riscv_opts.csr_check = false;
> > +  else if (strncmp (name, "arch,", 5) == 0)
> > +    {
> > +      name += 5;
> > +      if (ISSPACE (*name) && *name != '\0')
> > +     name++;
> > +      riscv_update_subset (&riscv_rps_as, name);
> > +    }
> >    else if (strcmp (name, "push") == 0)
> >      {
> >        struct riscv_option_stack *s;
> >
> > -      s = (struct riscv_option_stack *) xmalloc (sizeof *s);
> > +      s = XNEW (struct riscv_option_stack);
> >        s->next = riscv_opts_stack;
> >        s->options = riscv_opts;
> > +      s->subset_list = riscv_subsets;
> >        riscv_opts_stack = s;
> > +      riscv_subsets = riscv_copy_subset_list (s->subset_list);
> > +      riscv_rps_as.subset_list = riscv_subsets;
> >      }
> >    else if (strcmp (name, "pop") == 0)
> >      {
> > @@ -3776,8 +3795,12 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
> >       as_bad (_(".option pop with no .option push"));
> >        else
> >       {
> > -       riscv_opts = s->options;
> > +       riscv_subset_list_t *release_subsets = riscv_subsets;
> >         riscv_opts_stack = s->next;
> > +       riscv_opts = s->options;
> > +       riscv_subsets = s->subset_list;
> > +       riscv_rps_as.subset_list = riscv_subsets;
> > +       riscv_release_subset_list (release_subsets);
> >         free (s);
> >       }
> >      }
> > @@ -4262,7 +4285,7 @@ riscv_write_out_attrs (void)
> >    unsigned int i;
> >
> >    /* Re-write architecture elf attribute.  */
> > -  arch_str = riscv_arch_str (xlen, &riscv_subsets);
> > +  arch_str = riscv_arch_str (xlen, riscv_subsets);
> >    bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str);
> >    xfree ((void *) arch_str);
> >
> > diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
> > index bfbf61d..697be3a 100644
> > --- a/gas/doc/c-riscv.texi
> > +++ b/gas/doc/c-riscv.texi
> > @@ -194,7 +194,7 @@ command-line options are respected for the bulk of the file being assembled.
> >  @itemx norvc
> >  Enables or disables the generation of compressed instructions.  Instructions
> >  are opportunistically compressed by the RISC-V assembler when possible, but
> > -sometimes this behavior is not desirable.
> > +sometimes this behavior is not desirable, especially when handling alignments.
> >
> >  @item pic
> >  @itemx nopic
> > @@ -212,6 +212,19 @@ desirable.
> >  @itemx no-csr-check
> >  Enables or disables the CSR checking.
> >
> > +@item arch, @var{+extension[version]} [,...,@var{+extension_n[version_n]}]
> > +@itemx arch, @var{-extension} [,...,@var{-extension_n}]
> > +@itemx arch, @var{=ISA}
> > +Enables or disables the extensions for specific code region.  For example,
> > +@samp{.option arch, +m2p0} means add m extension with version 2.0, and
> > +@samp{.option arch, -f, -d} means remove extensions, f and d, from the
> > +architecture string.  Note that, @samp{.option arch, +c, -c} have the same
> > +behavior as @samp{.option rvc, norvc}.  However, they are also undesirable
> > +sometimes.  Besides, @samp{.option arch, -i} is illegal, since we cannot
> > +remove the base i extension anytime.  If you want to reset the whole ISA
> > +string, you can also use @samp{.option arch, =rv32imac} to overwrite the
> > +previous settings.
> > +
> >  @cindex INSN directives
> >  @item .insn @var{type}, @var{operand} [,...,@var{operand_n}]
> >  @itemx .insn @var{insn_length}, @var{value}
> > diff --git a/gas/testsuite/gas/riscv/option-arch-01.s b/gas/testsuite/gas/riscv/option-arch-01.s
> > new file mode 100644
> > index 0000000..201f9b3
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-01.s
> > @@ -0,0 +1,10 @@
> > +.attribute arch, "rv64ic"
> > +add  a0, a0, a1
> > +.option push
> > +.option arch, +d2p0, -c, +xvendor1p0
> > +add  a0, a0, a1
> > +frcsr        a0      # Should add mapping symbol with ISA here, and then dump it to frcsr.
> > +.option push
> > +.option arch, +i3p0, +m3p0, +d3p0
> > +.option pop
> > +.option pop
> > diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
> > new file mode 100644
> > index 0000000..59bc1d2
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-01a.d
> > @@ -0,0 +1,14 @@
> > +#as:
> > +#source: option-arch-01.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[    ]+[0-9a-f]+:[   ]+952e[         ]+add[          ]+a0,a0,a1
> > +[    ]+[0-9a-f]+:[   ]+00b50533[     ]+add[          ]+a0,a0,a1
> > +[    ]+[0-9a-f]+:[   ]+00302573[     ]+csrr[         ]+a0,fcsr
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-01b.d b/gas/testsuite/gas/riscv/option-arch-01b.d
> > new file mode 100644
> > index 0000000..9a6c2c5
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-01b.d
> > @@ -0,0 +1,8 @@
> > +#as:
> > +#readelf: -A
> > +#source: option-arch-01.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv64i2p0_c2p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-02.d b/gas/testsuite/gas/riscv/option-arch-02.d
> > new file mode 100644
> > index 0000000..0fe89ec
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-02.d
> > @@ -0,0 +1,8 @@
> > +#as:
> > +#readelf: -A
> > +#source: option-arch-02.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv64i3p0_m3p0_f2p0_d3p0_c2p0_xvendor32x3p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-02.s b/gas/testsuite/gas/riscv/option-arch-02.s
> > new file mode 100644
> > index 0000000..f4ceee84
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-02.s
> > @@ -0,0 +1,8 @@
> > +.attribute arch, "rv64ic"
> > +add  a0, a0, a1
> > +.option push
> > +.option arch, +d2p0, -c, +xvendor1p0
> > +add  a0, a0, a1
> > +frcsr        a0
> > +.option pop
> > +.option arch, +i3p0, +m3p0, +d3p0, +xvendor32x3p0
> > diff --git a/gas/testsuite/gas/riscv/option-arch-03.d b/gas/testsuite/gas/riscv/option-arch-03.d
> > new file mode 100644
> > index 0000000..b621d03
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-03.d
> > @@ -0,0 +1,8 @@
> > +#as:
> > +#readelf: -A
> > +#source: option-arch-03.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv32i2p0_c2p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/option-arch-03.s b/gas/testsuite/gas/riscv/option-arch-03.s
> > new file mode 100644
> > index 0000000..7183140
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-03.s
> > @@ -0,0 +1,3 @@
> > +.attribute arch, "rv64ic"
> > +.option arch, +d2p0, -c
> > +.option arch, =rv32ic
> > diff --git a/gas/testsuite/gas/riscv/option-arch-fail.d b/gas/testsuite/gas/riscv/option-arch-fail.d
> > new file mode 100644
> > index 0000000..bce5d32
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-fail.d
> > @@ -0,0 +1,3 @@
> > +#as:
> > +#source: option-arch-fail.s
> > +#error_output: option-arch-fail.l
> > diff --git a/gas/testsuite/gas/riscv/option-arch-fail.l b/gas/testsuite/gas/riscv/option-arch-fail.l
> > new file mode 100644
> > index 0000000..3e0599e
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-fail.l
> > @@ -0,0 +1,8 @@
> > +.*Assembler messages:
> > +.*Error: extensions must begin with \+/\-/\= in .option arch `m2p0'
> > +.*Error: cannot remove extension `i' in .option arch `\-i'
> > +.*Error: unknown ISA extension `zsubset' in .option arch `\+zsubset2p0'
> > +.*Error: unknown ISA extension `f2p0_d' in .option arch `\+f2p0_d2p0'
> > +.*Error: unknown ISA extension `' in .option arch `\+'
> > +.*Error: invalid ISA extension ends with <number>p in .option arch `\+xvendor2p'
> > +.*Error: .option pop with no .option push
> > diff --git a/gas/testsuite/gas/riscv/option-arch-fail.s b/gas/testsuite/gas/riscv/option-arch-fail.s
> > new file mode 100644
> > index 0000000..a0b1bde
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/option-arch-fail.s
> > @@ -0,0 +1,10 @@
> > +.attribute arch, "rv64ic"
> > +.option push
> > +.option arch, m2p0
> > +.option arch, -i
> > +.option arch, +zsubset2p0
> > +.option arch, +f2p0_d2p0
> > +.option arch, +
> > +.option arch, +xvendor2p
> > +.option pop
> > +.option pop


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-20  2:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 10:53 [committed v3] RISC-V: Support new .option arch directive Nelson Chu
2021-11-19 16:01 ` Palmer Dabbelt
2021-11-20  2:17   ` Nelson Chu
2021-11-20  2:32     ` Jessica Clarke

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