Re: [PATCH] RISC-V: Reorder the prefixed extensions which are out of order.

[Nelson Chu <nelson.chu@sifive.com>]

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
>