public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Reorder the prefixed extensions which are out of order.
@ 2021-05-31 12:37 Nelson Chu
  2021-06-03  5:56 ` Kito Cheng
  2022-06-22 10:43 ` Nelson Chu
  0 siblings, 2 replies; 5+ messages in thread
From: Nelson Chu @ 2021-05-31 12:37 UTC (permalink / raw)
  To: binutils, jimw, kito.cheng, andrew

bfd/
    * elfnn-riscv.c (riscv_merge_arch_attr_info): Updated.
    * elfxx-riscv.c (riscv_add_subset): Replaced riscv_add_subset
    with riscv_add_implicit_subset, and renamed it.
    (riscv_parse_add_subset): Updated.
    (riscv_release_subset_list): Likewise.
    (riscv_parse_prefixed_ext): Help to reorder the prefixed
    extensions if users set the wrong order.
    * elfxx-riscv.h (riscv_subset_t): Removed the unsed tail.
gas/
    * testsuite/gas/riscv/march-fail-order-x-z.d: Removed.
    * testsuite/gas/riscv/march-fail-order-x-z.l: Likewise.
    * testsuite/gas/riscv/march-fail-order-x.d: Likewise.
    * testsuite/gas/riscv/march-fail-order-x.l: Likewise.
    * testsuite/gas/riscv/march-fail-order-z.d: Likewise.
    * testsuite/gas/riscv/march-fail-order-z.l: Likewise.
    * testsuite/gas/riscv/attribute-prefixed-reorder.d: New testcase.
    Reorder the prefixed extensions which are out of order.
---
 bfd/elfnn-riscv.c                             |  1 -
 bfd/elfxx-riscv.c                             | 70 ++-----------------
 bfd/elfxx-riscv.h                             |  1 -
 .../gas/riscv/attribute-prefixed-reorder.d    |  7 ++
 .../gas/riscv/march-fail-order-x-z.d          |  3 -
 .../gas/riscv/march-fail-order-x-z.l          |  2 -
 gas/testsuite/gas/riscv/march-fail-order-x.d  |  3 -
 gas/testsuite/gas/riscv/march-fail-order-x.l  |  2 -
 gas/testsuite/gas/riscv/march-fail-order-z.d  |  3 -
 gas/testsuite/gas/riscv/march-fail-order-z.l  |  2 -
 10 files changed, 14 insertions(+), 80 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.d
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.l
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.d
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.l
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.d
 delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.l

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index eef1e800221..2ff1c6cada4 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -3551,7 +3551,6 @@ riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch)
 
   unsigned xlen_in, xlen_out;
   merged_subsets.head = NULL;
-  merged_subsets.tail = NULL;
 
   riscv_parse_subset_t rpe_in;
   riscv_parse_subset_t rpe_out;
diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 39b69e2b0a5..f7ccac77e81 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1294,37 +1294,14 @@ riscv_lookup_subset (const riscv_subset_list_t *subset_list,
   return false;
 }
 
-/* Add extension from ISA string to the last of the subset list.  */
+/* Add the extension to the subset list.  Search the
+   list first, and then find the right place to add.  */
 
 void
 riscv_add_subset (riscv_subset_list_t *subset_list,
 		  const char *subset,
 		  int major,
 		  int minor)
-{
-  riscv_subset_t *s = xmalloc (sizeof *s);
-
-  if (subset_list->head == NULL)
-    subset_list->head = s;
-
-  s->name = xstrdup (subset);
-  s->major_version = major;
-  s->minor_version = minor;
-  s->next = NULL;
-
-  if (subset_list->tail != NULL)
-    subset_list->tail->next = s;
-  subset_list->tail = s;
-}
-
-/* Add the implicit extension to the subset list.  Search the
-   list first, and then find the right place to add.  */
-
-static void
-riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
-			   const char *subset,
-			   int major,
-			   int minor)
 {
   riscv_subset_t *current, *new;
 
@@ -1349,12 +1326,7 @@ riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
     }
 }
 
-/* We have to add all extensions from ISA string first, and then start to
-   add their implicit extensions.  The extensions from ISA string must be
-   set in order, so we can add them to the last of the subset list
-   directly, without searching.
-
-   Find the default versions for the extension before adding them to
+/* Find the default versions for the extension before adding them to
    the subset list, if their versions are RISCV_UNKNOWN_VERSION.
    Afterwards, report errors if we can not find their default versions.  */
 
@@ -1389,12 +1361,8 @@ riscv_parse_add_subset (riscv_parse_subset_t *rps,
       return;
     }
 
-  if (!implicit)
-    riscv_add_subset (rps->subset_list, subset,
-		      major_version, minor_version);
-  else
-    riscv_add_implicit_subset (rps->subset_list, subset,
-			       major_version, minor_version);
+  riscv_add_subset (rps->subset_list, subset,
+		    major_version, minor_version);
 }
 
 /* Release subset list.  */
@@ -1409,8 +1377,6 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
       free (subset_list->head);
       subset_list->head = next;
     }
-
-  subset_list->tail = NULL;
 }
 
 /* Parsing extension version.
@@ -1636,7 +1602,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
 {
   int major_version;
   int minor_version;
-  const char *last_name;
   enum riscv_prefix_ext_class class;
 
   while (*p)
@@ -1691,28 +1656,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
 	  return NULL;
 	}
 
-      /* Check that the extension isn't duplicate.  */
-      last_name = rps->subset_list->tail->name;
-      if (!strcasecmp (last_name, subset))
-	{
-	  rps->error_handler
-	    (_("-march=%s: duplicate prefixed ISA extension `%s'"),
-	     march, subset);
-	  free (subset);
-	  return NULL;
-	}
-
-      /* Check that the extension is in expected order.  */
-      if (riscv_compare_subsets (last_name, subset) > 0)
-	{
-	  rps->error_handler
-	    (_("-march=%s: prefixed ISA extension `%s' is not in expected "
-	       "order.  It must come before `%s'"),
-	     march, subset, last_name);
-	  free (subset);
-	  return NULL;
-	}
-
       riscv_parse_add_subset (rps, subset,
 			      major_version,
 			      minor_version, false);
@@ -1811,7 +1754,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
   if (p == NULL)
     return false;
 
-  /* Parse the different classes of extensions in the specified order.  */
+  /* Parse the prefixed extensions, we will help to reorder them to
+     the correct order.  */
   while (*p != '\0')
     {
       p = riscv_parse_prefixed_ext (rps, arch, p);
diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
index 6a2501b7be8..c1885158a9f 100644
--- a/bfd/elfxx-riscv.h
+++ b/bfd/elfxx-riscv.h
@@ -49,7 +49,6 @@ typedef struct riscv_subset_t riscv_subset_t;
 typedef struct
 {
   riscv_subset_t *head;
-  riscv_subset_t *tail;
 } riscv_subset_list_t;
 
 extern void
diff --git a/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
new file mode 100644
index 00000000000..a46a68cd985
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
@@ -0,0 +1,7 @@
+#as: -misa-spec=20191213 -march-attr -march=rv64gc_zbc_zba_zifencei_xbargle2p0_zbb_zicsr_xargle2p0
+#readelf: -A
+#source: empty.s
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zba0p93_zbb0p93_zbc0p93_xargle2p0_xbargle2p0"
diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.d b/gas/testsuite/gas/riscv/march-fail-order-x-z.d
deleted file mode 100644
index 7245e68e0ea..00000000000
--- a/gas/testsuite/gas/riscv/march-fail-order-x-z.d
+++ /dev/null
@@ -1,3 +0,0 @@
-#as: -march=rv32i_xargle2p0_zicsr2p0
-#source: empty.s
-#error_output: march-fail-order-x-z.l
diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.l b/gas/testsuite/gas/riscv/march-fail-order-x-z.l
deleted file mode 100644
index 53ea8201187..00000000000
--- a/gas/testsuite/gas/riscv/march-fail-order-x-z.l
+++ /dev/null
@@ -1,2 +0,0 @@
-.*Assembler messages:
-.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `xargle'
diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.d b/gas/testsuite/gas/riscv/march-fail-order-x.d
deleted file mode 100644
index 72a821ef8e6..00000000000
--- a/gas/testsuite/gas/riscv/march-fail-order-x.d
+++ /dev/null
@@ -1,3 +0,0 @@
-#as: -march=rv32i_xbargle2p0_xargle2p0
-#source: empty.s
-#error_output: march-fail-order-x.l
diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.l b/gas/testsuite/gas/riscv/march-fail-order-x.l
deleted file mode 100644
index cfb118528df..00000000000
--- a/gas/testsuite/gas/riscv/march-fail-order-x.l
+++ /dev/null
@@ -1,2 +0,0 @@
-.*Assembler messages:
-.*Error: .*prefixed ISA extension `xargle' is not in expected order.  It must come before `xbargle'
diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.d b/gas/testsuite/gas/riscv/march-fail-order-z.d
deleted file mode 100644
index dd076c6d35a..00000000000
--- a/gas/testsuite/gas/riscv/march-fail-order-z.d
+++ /dev/null
@@ -1,3 +0,0 @@
-#as: -march=rv32i_zifencei2p0_zicsr2p0
-#source: empty.s
-#error_output: march-fail-order-z.l
diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.l b/gas/testsuite/gas/riscv/march-fail-order-z.l
deleted file mode 100644
index 468c412051f..00000000000
--- a/gas/testsuite/gas/riscv/march-fail-order-z.l
+++ /dev/null
@@ -1,2 +0,0 @@
-.*Assembler messages:
-.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `zifencei'
-- 
2.30.2


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

* Re: [PATCH] RISC-V: Reorder the prefixed extensions which are out of order.
  2021-05-31 12:37 [PATCH] RISC-V: Reorder the prefixed extensions which are out of order Nelson Chu
@ 2021-06-03  5:56 ` Kito Cheng
  2022-06-22 10:43 ` Nelson Chu
  1 sibling, 0 replies; 5+ messages in thread
From: Kito Cheng @ 2021-06-03  5:56 UTC (permalink / raw)
  To: Nelson Chu; +Cc: Binutils, Jim Wilson, Kito Cheng, Andrew Waterman

It's still on the RFC[1] stage, so I would suggest holding this patch
for a while.

[1] https://github.com/riscv/riscv-toolchain-conventions/issues/11

On Mon, May 31, 2021 at 8:50 PM Nelson Chu <nelson.chu@sifive.com> wrote:
>
> bfd/
>     * elfnn-riscv.c (riscv_merge_arch_attr_info): Updated.
>     * elfxx-riscv.c (riscv_add_subset): Replaced riscv_add_subset
>     with riscv_add_implicit_subset, and renamed it.
>     (riscv_parse_add_subset): Updated.
>     (riscv_release_subset_list): Likewise.
>     (riscv_parse_prefixed_ext): Help to reorder the prefixed
>     extensions if users set the wrong order.
>     * elfxx-riscv.h (riscv_subset_t): Removed the unsed tail.
> gas/
>     * testsuite/gas/riscv/march-fail-order-x-z.d: Removed.
>     * testsuite/gas/riscv/march-fail-order-x-z.l: Likewise.
>     * testsuite/gas/riscv/march-fail-order-x.d: Likewise.
>     * testsuite/gas/riscv/march-fail-order-x.l: Likewise.
>     * testsuite/gas/riscv/march-fail-order-z.d: Likewise.
>     * testsuite/gas/riscv/march-fail-order-z.l: Likewise.
>     * testsuite/gas/riscv/attribute-prefixed-reorder.d: New testcase.
>     Reorder the prefixed extensions which are out of order.
> ---
>  bfd/elfnn-riscv.c                             |  1 -
>  bfd/elfxx-riscv.c                             | 70 ++-----------------
>  bfd/elfxx-riscv.h                             |  1 -
>  .../gas/riscv/attribute-prefixed-reorder.d    |  7 ++
>  .../gas/riscv/march-fail-order-x-z.d          |  3 -
>  .../gas/riscv/march-fail-order-x-z.l          |  2 -
>  gas/testsuite/gas/riscv/march-fail-order-x.d  |  3 -
>  gas/testsuite/gas/riscv/march-fail-order-x.l  |  2 -
>  gas/testsuite/gas/riscv/march-fail-order-z.d  |  3 -
>  gas/testsuite/gas/riscv/march-fail-order-z.l  |  2 -
>  10 files changed, 14 insertions(+), 80 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.l
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.l
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.l
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index eef1e800221..2ff1c6cada4 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -3551,7 +3551,6 @@ riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch)
>
>    unsigned xlen_in, xlen_out;
>    merged_subsets.head = NULL;
> -  merged_subsets.tail = NULL;
>
>    riscv_parse_subset_t rpe_in;
>    riscv_parse_subset_t rpe_out;
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 39b69e2b0a5..f7ccac77e81 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1294,37 +1294,14 @@ riscv_lookup_subset (const riscv_subset_list_t *subset_list,
>    return false;
>  }
>
> -/* Add extension from ISA string to the last of the subset list.  */
> +/* Add the extension to the subset list.  Search the
> +   list first, and then find the right place to add.  */
>
>  void
>  riscv_add_subset (riscv_subset_list_t *subset_list,
>                   const char *subset,
>                   int major,
>                   int minor)
> -{
> -  riscv_subset_t *s = xmalloc (sizeof *s);
> -
> -  if (subset_list->head == NULL)
> -    subset_list->head = s;
> -
> -  s->name = xstrdup (subset);
> -  s->major_version = major;
> -  s->minor_version = minor;
> -  s->next = NULL;
> -
> -  if (subset_list->tail != NULL)
> -    subset_list->tail->next = s;
> -  subset_list->tail = s;
> -}
> -
> -/* Add the implicit extension to the subset list.  Search the
> -   list first, and then find the right place to add.  */
> -
> -static void
> -riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
> -                          const char *subset,
> -                          int major,
> -                          int minor)
>  {
>    riscv_subset_t *current, *new;
>
> @@ -1349,12 +1326,7 @@ riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
>      }
>  }
>
> -/* We have to add all extensions from ISA string first, and then start to
> -   add their implicit extensions.  The extensions from ISA string must be
> -   set in order, so we can add them to the last of the subset list
> -   directly, without searching.
> -
> -   Find the default versions for the extension before adding them to
> +/* Find the default versions for the extension before adding them to
>     the subset list, if their versions are RISCV_UNKNOWN_VERSION.
>     Afterwards, report errors if we can not find their default versions.  */
>
> @@ -1389,12 +1361,8 @@ riscv_parse_add_subset (riscv_parse_subset_t *rps,
>        return;
>      }
>
> -  if (!implicit)
> -    riscv_add_subset (rps->subset_list, subset,
> -                     major_version, minor_version);
> -  else
> -    riscv_add_implicit_subset (rps->subset_list, subset,
> -                              major_version, minor_version);
> +  riscv_add_subset (rps->subset_list, subset,
> +                   major_version, minor_version);
>  }
>
>  /* Release subset list.  */
> @@ -1409,8 +1377,6 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
>        free (subset_list->head);
>        subset_list->head = next;
>      }
> -
> -  subset_list->tail = NULL;
>  }
>
>  /* Parsing extension version.
> @@ -1636,7 +1602,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>  {
>    int major_version;
>    int minor_version;
> -  const char *last_name;
>    enum riscv_prefix_ext_class class;
>
>    while (*p)
> @@ -1691,28 +1656,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>           return NULL;
>         }
>
> -      /* Check that the extension isn't duplicate.  */
> -      last_name = rps->subset_list->tail->name;
> -      if (!strcasecmp (last_name, subset))
> -       {
> -         rps->error_handler
> -           (_("-march=%s: duplicate prefixed ISA extension `%s'"),
> -            march, subset);
> -         free (subset);
> -         return NULL;
> -       }
> -
> -      /* Check that the extension is in expected order.  */
> -      if (riscv_compare_subsets (last_name, subset) > 0)
> -       {
> -         rps->error_handler
> -           (_("-march=%s: prefixed ISA extension `%s' is not in expected "
> -              "order.  It must come before `%s'"),
> -            march, subset, last_name);
> -         free (subset);
> -         return NULL;
> -       }
> -
>        riscv_parse_add_subset (rps, subset,
>                               major_version,
>                               minor_version, false);
> @@ -1811,7 +1754,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
>    if (p == NULL)
>      return false;
>
> -  /* Parse the different classes of extensions in the specified order.  */
> +  /* Parse the prefixed extensions, we will help to reorder them to
> +     the correct order.  */
>    while (*p != '\0')
>      {
>        p = riscv_parse_prefixed_ext (rps, arch, p);
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index 6a2501b7be8..c1885158a9f 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -49,7 +49,6 @@ typedef struct riscv_subset_t riscv_subset_t;
>  typedef struct
>  {
>    riscv_subset_t *head;
> -  riscv_subset_t *tail;
>  } riscv_subset_list_t;
>
>  extern void
> diff --git a/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> new file mode 100644
> index 00000000000..a46a68cd985
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> @@ -0,0 +1,7 @@
> +#as: -misa-spec=20191213 -march-attr -march=rv64gc_zbc_zba_zifencei_xbargle2p0_zbb_zicsr_xargle2p0
> +#readelf: -A
> +#source: empty.s
> +
> +Attribute Section: riscv
> +File Attributes
> +  Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zba0p93_zbb0p93_zbc0p93_xargle2p0_xbargle2p0"
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.d b/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> deleted file mode 100644
> index 7245e68e0ea..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_xargle2p0_zicsr2p0
> -#source: empty.s
> -#error_output: march-fail-order-x-z.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.l b/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> deleted file mode 100644
> index 53ea8201187..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `xargle'
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.d b/gas/testsuite/gas/riscv/march-fail-order-x.d
> deleted file mode 100644
> index 72a821ef8e6..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_xbargle2p0_xargle2p0
> -#source: empty.s
> -#error_output: march-fail-order-x.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.l b/gas/testsuite/gas/riscv/march-fail-order-x.l
> deleted file mode 100644
> index cfb118528df..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*prefixed ISA extension `xargle' is not in expected order.  It must come before `xbargle'
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.d b/gas/testsuite/gas/riscv/march-fail-order-z.d
> deleted file mode 100644
> index dd076c6d35a..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-z.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_zifencei2p0_zicsr2p0
> -#source: empty.s
> -#error_output: march-fail-order-z.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.l b/gas/testsuite/gas/riscv/march-fail-order-z.l
> deleted file mode 100644
> index 468c412051f..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-z.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `zifencei'
> --
> 2.30.2
>

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

* Re: [PATCH] RISC-V: Reorder the prefixed extensions which are out of order.
  2021-05-31 12:37 [PATCH] RISC-V: Reorder the prefixed extensions which are out of order Nelson Chu
  2021-06-03  5:56 ` Kito Cheng
@ 2022-06-22 10:43 ` Nelson Chu
  2022-06-22 12:41   ` Kito Cheng
  1 sibling, 1 reply; 5+ messages in thread
From: Nelson Chu @ 2022-06-22 10:43 UTC (permalink / raw)
  To: Binutils, Jim Wilson, Kito Cheng, Andrew Waterman

Hi Guys,

I noticed that llvm can reorder the multi-letter extensions, even if
they are out of order in the architecture string.  Not really sure if
they can also reorder the single letter extensions, and not sure if
reordering the single extensions is a good idea.  Anyway, I think
RISC-V binuitls should at least help to reorder the multi-letters
extensions for now.  Therefore, I re-factor this patch and commit it.

Thanks
Nelson

On Mon, May 31, 2021 at 8:37 PM Nelson Chu <nelson.chu@sifive.com> wrote:
>
> bfd/
>     * elfnn-riscv.c (riscv_merge_arch_attr_info): Updated.
>     * elfxx-riscv.c (riscv_add_subset): Replaced riscv_add_subset
>     with riscv_add_implicit_subset, and renamed it.
>     (riscv_parse_add_subset): Updated.
>     (riscv_release_subset_list): Likewise.
>     (riscv_parse_prefixed_ext): Help to reorder the prefixed
>     extensions if users set the wrong order.
>     * elfxx-riscv.h (riscv_subset_t): Removed the unsed tail.
> gas/
>     * testsuite/gas/riscv/march-fail-order-x-z.d: Removed.
>     * testsuite/gas/riscv/march-fail-order-x-z.l: Likewise.
>     * testsuite/gas/riscv/march-fail-order-x.d: Likewise.
>     * testsuite/gas/riscv/march-fail-order-x.l: Likewise.
>     * testsuite/gas/riscv/march-fail-order-z.d: Likewise.
>     * testsuite/gas/riscv/march-fail-order-z.l: Likewise.
>     * testsuite/gas/riscv/attribute-prefixed-reorder.d: New testcase.
>     Reorder the prefixed extensions which are out of order.
> ---
>  bfd/elfnn-riscv.c                             |  1 -
>  bfd/elfxx-riscv.c                             | 70 ++-----------------
>  bfd/elfxx-riscv.h                             |  1 -
>  .../gas/riscv/attribute-prefixed-reorder.d    |  7 ++
>  .../gas/riscv/march-fail-order-x-z.d          |  3 -
>  .../gas/riscv/march-fail-order-x-z.l          |  2 -
>  gas/testsuite/gas/riscv/march-fail-order-x.d  |  3 -
>  gas/testsuite/gas/riscv/march-fail-order-x.l  |  2 -
>  gas/testsuite/gas/riscv/march-fail-order-z.d  |  3 -
>  gas/testsuite/gas/riscv/march-fail-order-z.l  |  2 -
>  10 files changed, 14 insertions(+), 80 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.l
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.l
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.l
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index eef1e800221..2ff1c6cada4 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -3551,7 +3551,6 @@ riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch)
>
>    unsigned xlen_in, xlen_out;
>    merged_subsets.head = NULL;
> -  merged_subsets.tail = NULL;
>
>    riscv_parse_subset_t rpe_in;
>    riscv_parse_subset_t rpe_out;
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 39b69e2b0a5..f7ccac77e81 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1294,37 +1294,14 @@ riscv_lookup_subset (const riscv_subset_list_t *subset_list,
>    return false;
>  }
>
> -/* Add extension from ISA string to the last of the subset list.  */
> +/* Add the extension to the subset list.  Search the
> +   list first, and then find the right place to add.  */
>
>  void
>  riscv_add_subset (riscv_subset_list_t *subset_list,
>                   const char *subset,
>                   int major,
>                   int minor)
> -{
> -  riscv_subset_t *s = xmalloc (sizeof *s);
> -
> -  if (subset_list->head == NULL)
> -    subset_list->head = s;
> -
> -  s->name = xstrdup (subset);
> -  s->major_version = major;
> -  s->minor_version = minor;
> -  s->next = NULL;
> -
> -  if (subset_list->tail != NULL)
> -    subset_list->tail->next = s;
> -  subset_list->tail = s;
> -}
> -
> -/* Add the implicit extension to the subset list.  Search the
> -   list first, and then find the right place to add.  */
> -
> -static void
> -riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
> -                          const char *subset,
> -                          int major,
> -                          int minor)
>  {
>    riscv_subset_t *current, *new;
>
> @@ -1349,12 +1326,7 @@ riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
>      }
>  }
>
> -/* We have to add all extensions from ISA string first, and then start to
> -   add their implicit extensions.  The extensions from ISA string must be
> -   set in order, so we can add them to the last of the subset list
> -   directly, without searching.
> -
> -   Find the default versions for the extension before adding them to
> +/* Find the default versions for the extension before adding them to
>     the subset list, if their versions are RISCV_UNKNOWN_VERSION.
>     Afterwards, report errors if we can not find their default versions.  */
>
> @@ -1389,12 +1361,8 @@ riscv_parse_add_subset (riscv_parse_subset_t *rps,
>        return;
>      }
>
> -  if (!implicit)
> -    riscv_add_subset (rps->subset_list, subset,
> -                     major_version, minor_version);
> -  else
> -    riscv_add_implicit_subset (rps->subset_list, subset,
> -                              major_version, minor_version);
> +  riscv_add_subset (rps->subset_list, subset,
> +                   major_version, minor_version);
>  }
>
>  /* Release subset list.  */
> @@ -1409,8 +1377,6 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
>        free (subset_list->head);
>        subset_list->head = next;
>      }
> -
> -  subset_list->tail = NULL;
>  }
>
>  /* Parsing extension version.
> @@ -1636,7 +1602,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>  {
>    int major_version;
>    int minor_version;
> -  const char *last_name;
>    enum riscv_prefix_ext_class class;
>
>    while (*p)
> @@ -1691,28 +1656,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>           return NULL;
>         }
>
> -      /* Check that the extension isn't duplicate.  */
> -      last_name = rps->subset_list->tail->name;
> -      if (!strcasecmp (last_name, subset))
> -       {
> -         rps->error_handler
> -           (_("-march=%s: duplicate prefixed ISA extension `%s'"),
> -            march, subset);
> -         free (subset);
> -         return NULL;
> -       }
> -
> -      /* Check that the extension is in expected order.  */
> -      if (riscv_compare_subsets (last_name, subset) > 0)
> -       {
> -         rps->error_handler
> -           (_("-march=%s: prefixed ISA extension `%s' is not in expected "
> -              "order.  It must come before `%s'"),
> -            march, subset, last_name);
> -         free (subset);
> -         return NULL;
> -       }
> -
>        riscv_parse_add_subset (rps, subset,
>                               major_version,
>                               minor_version, false);
> @@ -1811,7 +1754,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
>    if (p == NULL)
>      return false;
>
> -  /* Parse the different classes of extensions in the specified order.  */
> +  /* Parse the prefixed extensions, we will help to reorder them to
> +     the correct order.  */
>    while (*p != '\0')
>      {
>        p = riscv_parse_prefixed_ext (rps, arch, p);
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index 6a2501b7be8..c1885158a9f 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -49,7 +49,6 @@ typedef struct riscv_subset_t riscv_subset_t;
>  typedef struct
>  {
>    riscv_subset_t *head;
> -  riscv_subset_t *tail;
>  } riscv_subset_list_t;
>
>  extern void
> diff --git a/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> new file mode 100644
> index 00000000000..a46a68cd985
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> @@ -0,0 +1,7 @@
> +#as: -misa-spec=20191213 -march-attr -march=rv64gc_zbc_zba_zifencei_xbargle2p0_zbb_zicsr_xargle2p0
> +#readelf: -A
> +#source: empty.s
> +
> +Attribute Section: riscv
> +File Attributes
> +  Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zba0p93_zbb0p93_zbc0p93_xargle2p0_xbargle2p0"
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.d b/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> deleted file mode 100644
> index 7245e68e0ea..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_xargle2p0_zicsr2p0
> -#source: empty.s
> -#error_output: march-fail-order-x-z.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.l b/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> deleted file mode 100644
> index 53ea8201187..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `xargle'
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.d b/gas/testsuite/gas/riscv/march-fail-order-x.d
> deleted file mode 100644
> index 72a821ef8e6..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_xbargle2p0_xargle2p0
> -#source: empty.s
> -#error_output: march-fail-order-x.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.l b/gas/testsuite/gas/riscv/march-fail-order-x.l
> deleted file mode 100644
> index cfb118528df..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*prefixed ISA extension `xargle' is not in expected order.  It must come before `xbargle'
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.d b/gas/testsuite/gas/riscv/march-fail-order-z.d
> deleted file mode 100644
> index dd076c6d35a..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-z.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_zifencei2p0_zicsr2p0
> -#source: empty.s
> -#error_output: march-fail-order-z.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.l b/gas/testsuite/gas/riscv/march-fail-order-z.l
> deleted file mode 100644
> index 468c412051f..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-z.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `zifencei'
> --
> 2.30.2
>

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

* Re: [PATCH] RISC-V: Reorder the prefixed extensions which are out of order.
  2022-06-22 10:43 ` Nelson Chu
@ 2022-06-22 12:41   ` Kito Cheng
  2022-06-24  0:38     ` Nelson Chu
  0 siblings, 1 reply; 5+ messages in thread
From: Kito Cheng @ 2022-06-22 12:41 UTC (permalink / raw)
  To: Nelson Chu; +Cc: Binutils, Jim Wilson, Andrew Waterman

> I noticed that llvm can reorder the multi-letter extensions, even if
> they are out of order in the architecture string.

Wait, IIRC that's our downstream change for clang...

> Not really sure if
> they can also reorder the single letter extensions, and not sure if
> reordering the single extensions is a good idea.  Anyway, I think
> RISC-V binuitls should at least help to reorder the multi-letters
> extensions for now.  Therefore, I re-factor this patch and commit it.
>
> Thanks
> Nelson
>
> On Mon, May 31, 2021 at 8:37 PM Nelson Chu <nelson.chu@sifive.com> wrote:
> >
> > bfd/
> >     * elfnn-riscv.c (riscv_merge_arch_attr_info): Updated.
> >     * elfxx-riscv.c (riscv_add_subset): Replaced riscv_add_subset
> >     with riscv_add_implicit_subset, and renamed it.
> >     (riscv_parse_add_subset): Updated.
> >     (riscv_release_subset_list): Likewise.
> >     (riscv_parse_prefixed_ext): Help to reorder the prefixed
> >     extensions if users set the wrong order.
> >     * elfxx-riscv.h (riscv_subset_t): Removed the unsed tail.
> > gas/
> >     * testsuite/gas/riscv/march-fail-order-x-z.d: Removed.
> >     * testsuite/gas/riscv/march-fail-order-x-z.l: Likewise.
> >     * testsuite/gas/riscv/march-fail-order-x.d: Likewise.
> >     * testsuite/gas/riscv/march-fail-order-x.l: Likewise.
> >     * testsuite/gas/riscv/march-fail-order-z.d: Likewise.
> >     * testsuite/gas/riscv/march-fail-order-z.l: Likewise.
> >     * testsuite/gas/riscv/attribute-prefixed-reorder.d: New testcase.
> >     Reorder the prefixed extensions which are out of order.
> > ---
> >  bfd/elfnn-riscv.c                             |  1 -
> >  bfd/elfxx-riscv.c                             | 70 ++-----------------
> >  bfd/elfxx-riscv.h                             |  1 -
> >  .../gas/riscv/attribute-prefixed-reorder.d    |  7 ++
> >  .../gas/riscv/march-fail-order-x-z.d          |  3 -
> >  .../gas/riscv/march-fail-order-x-z.l          |  2 -
> >  gas/testsuite/gas/riscv/march-fail-order-x.d  |  3 -
> >  gas/testsuite/gas/riscv/march-fail-order-x.l  |  2 -
> >  gas/testsuite/gas/riscv/march-fail-order-z.d  |  3 -
> >  gas/testsuite/gas/riscv/march-fail-order-z.l  |  2 -
> >  10 files changed, 14 insertions(+), 80 deletions(-)
> >  create mode 100644 gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.d
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.l
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.d
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.l
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.d
> >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.l
> >
> > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> > index eef1e800221..2ff1c6cada4 100644
> > --- a/bfd/elfnn-riscv.c
> > +++ b/bfd/elfnn-riscv.c
> > @@ -3551,7 +3551,6 @@ riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch)
> >
> >    unsigned xlen_in, xlen_out;
> >    merged_subsets.head = NULL;
> > -  merged_subsets.tail = NULL;
> >
> >    riscv_parse_subset_t rpe_in;
> >    riscv_parse_subset_t rpe_out;
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index 39b69e2b0a5..f7ccac77e81 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1294,37 +1294,14 @@ riscv_lookup_subset (const riscv_subset_list_t *subset_list,
> >    return false;
> >  }
> >
> > -/* Add extension from ISA string to the last of the subset list.  */
> > +/* Add the extension to the subset list.  Search the
> > +   list first, and then find the right place to add.  */
> >
> >  void
> >  riscv_add_subset (riscv_subset_list_t *subset_list,
> >                   const char *subset,
> >                   int major,
> >                   int minor)
> > -{
> > -  riscv_subset_t *s = xmalloc (sizeof *s);
> > -
> > -  if (subset_list->head == NULL)
> > -    subset_list->head = s;
> > -
> > -  s->name = xstrdup (subset);
> > -  s->major_version = major;
> > -  s->minor_version = minor;
> > -  s->next = NULL;
> > -
> > -  if (subset_list->tail != NULL)
> > -    subset_list->tail->next = s;
> > -  subset_list->tail = s;
> > -}
> > -
> > -/* Add the implicit extension to the subset list.  Search the
> > -   list first, and then find the right place to add.  */
> > -
> > -static void
> > -riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
> > -                          const char *subset,
> > -                          int major,
> > -                          int minor)
> >  {
> >    riscv_subset_t *current, *new;
> >
> > @@ -1349,12 +1326,7 @@ riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
> >      }
> >  }
> >
> > -/* We have to add all extensions from ISA string first, and then start to
> > -   add their implicit extensions.  The extensions from ISA string must be
> > -   set in order, so we can add them to the last of the subset list
> > -   directly, without searching.
> > -
> > -   Find the default versions for the extension before adding them to
> > +/* Find the default versions for the extension before adding them to
> >     the subset list, if their versions are RISCV_UNKNOWN_VERSION.
> >     Afterwards, report errors if we can not find their default versions.  */
> >
> > @@ -1389,12 +1361,8 @@ riscv_parse_add_subset (riscv_parse_subset_t *rps,
> >        return;
> >      }
> >
> > -  if (!implicit)
> > -    riscv_add_subset (rps->subset_list, subset,
> > -                     major_version, minor_version);
> > -  else
> > -    riscv_add_implicit_subset (rps->subset_list, subset,
> > -                              major_version, minor_version);
> > +  riscv_add_subset (rps->subset_list, subset,
> > +                   major_version, minor_version);
> >  }
> >
> >  /* Release subset list.  */
> > @@ -1409,8 +1377,6 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
> >        free (subset_list->head);
> >        subset_list->head = next;
> >      }
> > -
> > -  subset_list->tail = NULL;
> >  }
> >
> >  /* Parsing extension version.
> > @@ -1636,7 +1602,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
> >  {
> >    int major_version;
> >    int minor_version;
> > -  const char *last_name;
> >    enum riscv_prefix_ext_class class;
> >
> >    while (*p)
> > @@ -1691,28 +1656,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
> >           return NULL;
> >         }
> >
> > -      /* Check that the extension isn't duplicate.  */
> > -      last_name = rps->subset_list->tail->name;
> > -      if (!strcasecmp (last_name, subset))
> > -       {
> > -         rps->error_handler
> > -           (_("-march=%s: duplicate prefixed ISA extension `%s'"),
> > -            march, subset);
> > -         free (subset);
> > -         return NULL;
> > -       }
> > -
> > -      /* Check that the extension is in expected order.  */
> > -      if (riscv_compare_subsets (last_name, subset) > 0)
> > -       {
> > -         rps->error_handler
> > -           (_("-march=%s: prefixed ISA extension `%s' is not in expected "
> > -              "order.  It must come before `%s'"),
> > -            march, subset, last_name);
> > -         free (subset);
> > -         return NULL;
> > -       }
> > -
> >        riscv_parse_add_subset (rps, subset,
> >                               major_version,
> >                               minor_version, false);
> > @@ -1811,7 +1754,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
> >    if (p == NULL)
> >      return false;
> >
> > -  /* Parse the different classes of extensions in the specified order.  */
> > +  /* Parse the prefixed extensions, we will help to reorder them to
> > +     the correct order.  */
> >    while (*p != '\0')
> >      {
> >        p = riscv_parse_prefixed_ext (rps, arch, p);
> > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> > index 6a2501b7be8..c1885158a9f 100644
> > --- a/bfd/elfxx-riscv.h
> > +++ b/bfd/elfxx-riscv.h
> > @@ -49,7 +49,6 @@ typedef struct riscv_subset_t riscv_subset_t;
> >  typedef struct
> >  {
> >    riscv_subset_t *head;
> > -  riscv_subset_t *tail;
> >  } riscv_subset_list_t;
> >
> >  extern void
> > diff --git a/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> > new file mode 100644
> > index 00000000000..a46a68cd985
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> > @@ -0,0 +1,7 @@
> > +#as: -misa-spec=20191213 -march-attr -march=rv64gc_zbc_zba_zifencei_xbargle2p0_zbb_zicsr_xargle2p0
> > +#readelf: -A
> > +#source: empty.s
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > +  Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zba0p93_zbb0p93_zbc0p93_xargle2p0_xbargle2p0"
> > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.d b/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> > deleted file mode 100644
> > index 7245e68e0ea..00000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -#as: -march=rv32i_xargle2p0_zicsr2p0
> > -#source: empty.s
> > -#error_output: march-fail-order-x-z.l
> > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.l b/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> > deleted file mode 100644
> > index 53ea8201187..00000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> > +++ /dev/null
> > @@ -1,2 +0,0 @@
> > -.*Assembler messages:
> > -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `xargle'
> > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.d b/gas/testsuite/gas/riscv/march-fail-order-x.d
> > deleted file mode 100644
> > index 72a821ef8e6..00000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-order-x.d
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -#as: -march=rv32i_xbargle2p0_xargle2p0
> > -#source: empty.s
> > -#error_output: march-fail-order-x.l
> > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.l b/gas/testsuite/gas/riscv/march-fail-order-x.l
> > deleted file mode 100644
> > index cfb118528df..00000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-order-x.l
> > +++ /dev/null
> > @@ -1,2 +0,0 @@
> > -.*Assembler messages:
> > -.*Error: .*prefixed ISA extension `xargle' is not in expected order.  It must come before `xbargle'
> > diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.d b/gas/testsuite/gas/riscv/march-fail-order-z.d
> > deleted file mode 100644
> > index dd076c6d35a..00000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-order-z.d
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -#as: -march=rv32i_zifencei2p0_zicsr2p0
> > -#source: empty.s
> > -#error_output: march-fail-order-z.l
> > diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.l b/gas/testsuite/gas/riscv/march-fail-order-z.l
> > deleted file mode 100644
> > index 468c412051f..00000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-order-z.l
> > +++ /dev/null
> > @@ -1,2 +0,0 @@
> > -.*Assembler messages:
> > -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `zifencei'
> > --
> > 2.30.2
> >

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

* Re: [PATCH] RISC-V: Reorder the prefixed extensions which are out of order.
  2022-06-22 12:41   ` Kito Cheng
@ 2022-06-24  0:38     ` Nelson Chu
  0 siblings, 0 replies; 5+ messages in thread
From: Nelson Chu @ 2022-06-24  0:38 UTC (permalink / raw)
  To: Kito Cheng; +Cc: Binutils, Jim Wilson, Andrew Waterman

Oops... ummmmm... Fortunately, according to the following link, more
people prefer to relax the architecture string order for both single
and multiple extensions, which means tools should help to reorder all
extensions for -march option.  Although it was an accident, it was
half done, and I pinged this issue again by the way...

https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11

Thanks
Nelson

On Wed, Jun 22, 2022 at 8:41 PM Kito Cheng <kito.cheng@sifive.com> wrote:
>
> > I noticed that llvm can reorder the multi-letter extensions, even if
> > they are out of order in the architecture string.
>
> Wait, IIRC that's our downstream change for clang...
>
> > Not really sure if
> > they can also reorder the single letter extensions, and not sure if
> > reordering the single extensions is a good idea.  Anyway, I think
> > RISC-V binuitls should at least help to reorder the multi-letters
> > extensions for now.  Therefore, I re-factor this patch and commit it.
> >
> > Thanks
> > Nelson
> >
> > On Mon, May 31, 2021 at 8:37 PM Nelson Chu <nelson.chu@sifive.com> wrote:
> > >
> > > bfd/
> > >     * elfnn-riscv.c (riscv_merge_arch_attr_info): Updated.
> > >     * elfxx-riscv.c (riscv_add_subset): Replaced riscv_add_subset
> > >     with riscv_add_implicit_subset, and renamed it.
> > >     (riscv_parse_add_subset): Updated.
> > >     (riscv_release_subset_list): Likewise.
> > >     (riscv_parse_prefixed_ext): Help to reorder the prefixed
> > >     extensions if users set the wrong order.
> > >     * elfxx-riscv.h (riscv_subset_t): Removed the unsed tail.
> > > gas/
> > >     * testsuite/gas/riscv/march-fail-order-x-z.d: Removed.
> > >     * testsuite/gas/riscv/march-fail-order-x-z.l: Likewise.
> > >     * testsuite/gas/riscv/march-fail-order-x.d: Likewise.
> > >     * testsuite/gas/riscv/march-fail-order-x.l: Likewise.
> > >     * testsuite/gas/riscv/march-fail-order-z.d: Likewise.
> > >     * testsuite/gas/riscv/march-fail-order-z.l: Likewise.
> > >     * testsuite/gas/riscv/attribute-prefixed-reorder.d: New testcase.
> > >     Reorder the prefixed extensions which are out of order.
> > > ---
> > >  bfd/elfnn-riscv.c                             |  1 -
> > >  bfd/elfxx-riscv.c                             | 70 ++-----------------
> > >  bfd/elfxx-riscv.h                             |  1 -
> > >  .../gas/riscv/attribute-prefixed-reorder.d    |  7 ++
> > >  .../gas/riscv/march-fail-order-x-z.d          |  3 -
> > >  .../gas/riscv/march-fail-order-x-z.l          |  2 -
> > >  gas/testsuite/gas/riscv/march-fail-order-x.d  |  3 -
> > >  gas/testsuite/gas/riscv/march-fail-order-x.l  |  2 -
> > >  gas/testsuite/gas/riscv/march-fail-order-z.d  |  3 -
> > >  gas/testsuite/gas/riscv/march-fail-order-z.l  |  2 -
> > >  10 files changed, 14 insertions(+), 80 deletions(-)
> > >  create mode 100644 gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> > >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.d
> > >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-z.l
> > >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.d
> > >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x.l
> > >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.d
> > >  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z.l
> > >
> > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> > > index eef1e800221..2ff1c6cada4 100644
> > > --- a/bfd/elfnn-riscv.c
> > > +++ b/bfd/elfnn-riscv.c
> > > @@ -3551,7 +3551,6 @@ riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch)
> > >
> > >    unsigned xlen_in, xlen_out;
> > >    merged_subsets.head = NULL;
> > > -  merged_subsets.tail = NULL;
> > >
> > >    riscv_parse_subset_t rpe_in;
> > >    riscv_parse_subset_t rpe_out;
> > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > > index 39b69e2b0a5..f7ccac77e81 100644
> > > --- a/bfd/elfxx-riscv.c
> > > +++ b/bfd/elfxx-riscv.c
> > > @@ -1294,37 +1294,14 @@ riscv_lookup_subset (const riscv_subset_list_t *subset_list,
> > >    return false;
> > >  }
> > >
> > > -/* Add extension from ISA string to the last of the subset list.  */
> > > +/* Add the extension to the subset list.  Search the
> > > +   list first, and then find the right place to add.  */
> > >
> > >  void
> > >  riscv_add_subset (riscv_subset_list_t *subset_list,
> > >                   const char *subset,
> > >                   int major,
> > >                   int minor)
> > > -{
> > > -  riscv_subset_t *s = xmalloc (sizeof *s);
> > > -
> > > -  if (subset_list->head == NULL)
> > > -    subset_list->head = s;
> > > -
> > > -  s->name = xstrdup (subset);
> > > -  s->major_version = major;
> > > -  s->minor_version = minor;
> > > -  s->next = NULL;
> > > -
> > > -  if (subset_list->tail != NULL)
> > > -    subset_list->tail->next = s;
> > > -  subset_list->tail = s;
> > > -}
> > > -
> > > -/* Add the implicit extension to the subset list.  Search the
> > > -   list first, and then find the right place to add.  */
> > > -
> > > -static void
> > > -riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
> > > -                          const char *subset,
> > > -                          int major,
> > > -                          int minor)
> > >  {
> > >    riscv_subset_t *current, *new;
> > >
> > > @@ -1349,12 +1326,7 @@ riscv_add_implicit_subset (riscv_subset_list_t *subset_list,
> > >      }
> > >  }
> > >
> > > -/* We have to add all extensions from ISA string first, and then start to
> > > -   add their implicit extensions.  The extensions from ISA string must be
> > > -   set in order, so we can add them to the last of the subset list
> > > -   directly, without searching.
> > > -
> > > -   Find the default versions for the extension before adding them to
> > > +/* Find the default versions for the extension before adding them to
> > >     the subset list, if their versions are RISCV_UNKNOWN_VERSION.
> > >     Afterwards, report errors if we can not find their default versions.  */
> > >
> > > @@ -1389,12 +1361,8 @@ riscv_parse_add_subset (riscv_parse_subset_t *rps,
> > >        return;
> > >      }
> > >
> > > -  if (!implicit)
> > > -    riscv_add_subset (rps->subset_list, subset,
> > > -                     major_version, minor_version);
> > > -  else
> > > -    riscv_add_implicit_subset (rps->subset_list, subset,
> > > -                              major_version, minor_version);
> > > +  riscv_add_subset (rps->subset_list, subset,
> > > +                   major_version, minor_version);
> > >  }
> > >
> > >  /* Release subset list.  */
> > > @@ -1409,8 +1377,6 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
> > >        free (subset_list->head);
> > >        subset_list->head = next;
> > >      }
> > > -
> > > -  subset_list->tail = NULL;
> > >  }
> > >
> > >  /* Parsing extension version.
> > > @@ -1636,7 +1602,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
> > >  {
> > >    int major_version;
> > >    int minor_version;
> > > -  const char *last_name;
> > >    enum riscv_prefix_ext_class class;
> > >
> > >    while (*p)
> > > @@ -1691,28 +1656,6 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
> > >           return NULL;
> > >         }
> > >
> > > -      /* Check that the extension isn't duplicate.  */
> > > -      last_name = rps->subset_list->tail->name;
> > > -      if (!strcasecmp (last_name, subset))
> > > -       {
> > > -         rps->error_handler
> > > -           (_("-march=%s: duplicate prefixed ISA extension `%s'"),
> > > -            march, subset);
> > > -         free (subset);
> > > -         return NULL;
> > > -       }
> > > -
> > > -      /* Check that the extension is in expected order.  */
> > > -      if (riscv_compare_subsets (last_name, subset) > 0)
> > > -       {
> > > -         rps->error_handler
> > > -           (_("-march=%s: prefixed ISA extension `%s' is not in expected "
> > > -              "order.  It must come before `%s'"),
> > > -            march, subset, last_name);
> > > -         free (subset);
> > > -         return NULL;
> > > -       }
> > > -
> > >        riscv_parse_add_subset (rps, subset,
> > >                               major_version,
> > >                               minor_version, false);
> > > @@ -1811,7 +1754,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
> > >    if (p == NULL)
> > >      return false;
> > >
> > > -  /* Parse the different classes of extensions in the specified order.  */
> > > +  /* Parse the prefixed extensions, we will help to reorder them to
> > > +     the correct order.  */
> > >    while (*p != '\0')
> > >      {
> > >        p = riscv_parse_prefixed_ext (rps, arch, p);
> > > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> > > index 6a2501b7be8..c1885158a9f 100644
> > > --- a/bfd/elfxx-riscv.h
> > > +++ b/bfd/elfxx-riscv.h
> > > @@ -49,7 +49,6 @@ typedef struct riscv_subset_t riscv_subset_t;
> > >  typedef struct
> > >  {
> > >    riscv_subset_t *head;
> > > -  riscv_subset_t *tail;
> > >  } riscv_subset_list_t;
> > >
> > >  extern void
> > > diff --git a/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> > > new file mode 100644
> > > index 00000000000..a46a68cd985
> > > --- /dev/null
> > > +++ b/gas/testsuite/gas/riscv/attribute-prefixed-reorder.d
> > > @@ -0,0 +1,7 @@
> > > +#as: -misa-spec=20191213 -march-attr -march=rv64gc_zbc_zba_zifencei_xbargle2p0_zbb_zicsr_xargle2p0
> > > +#readelf: -A
> > > +#source: empty.s
> > > +
> > > +Attribute Section: riscv
> > > +File Attributes
> > > +  Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zba0p93_zbb0p93_zbc0p93_xargle2p0_xbargle2p0"
> > > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.d b/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> > > deleted file mode 100644
> > > index 7245e68e0ea..00000000000
> > > --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.d
> > > +++ /dev/null
> > > @@ -1,3 +0,0 @@
> > > -#as: -march=rv32i_xargle2p0_zicsr2p0
> > > -#source: empty.s
> > > -#error_output: march-fail-order-x-z.l
> > > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-z.l b/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> > > deleted file mode 100644
> > > index 53ea8201187..00000000000
> > > --- a/gas/testsuite/gas/riscv/march-fail-order-x-z.l
> > > +++ /dev/null
> > > @@ -1,2 +0,0 @@
> > > -.*Assembler messages:
> > > -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `xargle'
> > > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.d b/gas/testsuite/gas/riscv/march-fail-order-x.d
> > > deleted file mode 100644
> > > index 72a821ef8e6..00000000000
> > > --- a/gas/testsuite/gas/riscv/march-fail-order-x.d
> > > +++ /dev/null
> > > @@ -1,3 +0,0 @@
> > > -#as: -march=rv32i_xbargle2p0_xargle2p0
> > > -#source: empty.s
> > > -#error_output: march-fail-order-x.l
> > > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x.l b/gas/testsuite/gas/riscv/march-fail-order-x.l
> > > deleted file mode 100644
> > > index cfb118528df..00000000000
> > > --- a/gas/testsuite/gas/riscv/march-fail-order-x.l
> > > +++ /dev/null
> > > @@ -1,2 +0,0 @@
> > > -.*Assembler messages:
> > > -.*Error: .*prefixed ISA extension `xargle' is not in expected order.  It must come before `xbargle'
> > > diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.d b/gas/testsuite/gas/riscv/march-fail-order-z.d
> > > deleted file mode 100644
> > > index dd076c6d35a..00000000000
> > > --- a/gas/testsuite/gas/riscv/march-fail-order-z.d
> > > +++ /dev/null
> > > @@ -1,3 +0,0 @@
> > > -#as: -march=rv32i_zifencei2p0_zicsr2p0
> > > -#source: empty.s
> > > -#error_output: march-fail-order-z.l
> > > diff --git a/gas/testsuite/gas/riscv/march-fail-order-z.l b/gas/testsuite/gas/riscv/march-fail-order-z.l
> > > deleted file mode 100644
> > > index 468c412051f..00000000000
> > > --- a/gas/testsuite/gas/riscv/march-fail-order-z.l
> > > +++ /dev/null
> > > @@ -1,2 +0,0 @@
> > > -.*Assembler messages:
> > > -.*Error: .*prefixed ISA extension `zicsr' is not in expected order.  It must come before `zifencei'
> > > --
> > > 2.30.2
> > >

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

end of thread, other threads:[~2022-06-24  0:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31 12:37 [PATCH] RISC-V: Reorder the prefixed extensions which are out of order Nelson Chu
2021-06-03  5:56 ` Kito Cheng
2022-06-22 10:43 ` Nelson Chu
2022-06-22 12:41   ` Kito Cheng
2022-06-24  0:38     ` Nelson Chu

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