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

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

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
>