[PATCH] RISC-V: Relax the order checking for the architecture string.

Kito Cheng kito.cheng@gmail.com
Thu Dec 22 09:38:46 GMT 2022


LGTM, thanks!

On Wed, Dec 21, 2022 at 8:10 PM Nelson Chu <nelson@rivosinc.com> wrote:
>
> * riscv-toolchain-conventions,
> PR, https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14
> Issue, https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11
>
> * Refer to the commit afc41ffb,
> RISC-V: Reorder the prefixed extensions which are out of order.
>
> In the past we only allow to reorder the prefixed extensions.  But according
> to the PR 14 in the riscv-toolchain-convention, we can also relax the order
> checking to allow the whole extensions be written out of orders, including
> the single standard extensions and the prefixed multi-letter extensions.
> Just that we still need to follow the following rules as usual,
>
> 1. prefixed extensions need to be seperated with `_'.
> 2. prefixed extensions need complete <major>.<minor> version if set.
>
> Please see the details in the march-ok-reorder gas testcase.
>
> Passed the riscv-gnu-toolchain regressions.
>
> bfd/
>     * elfxx-riscv.c (enum riscv_prefix_ext_class): Changed RV_ISA_CLASS_UNKNOWN
>     to RV_ISA_CLASS_SINGLE, since everything that does not belong to the
>     multi-keyword will possible be a single extension for the current parser.
>     (parse_config): Likewise.
>     (riscv_get_prefix_class): Likewise.
>     (riscv_compare_subsets): Likewise.
>     (riscv_parse_std_ext): Removed, and merged with riscv_parse_prefixed_ext
>     into riscv_parse_extensions.
>     (riscv_parse_prefixed_ext): Likewise.
>     (riscv_parse_subset): Only need to call riscv_parse_extensions to parse
>     both single standard and prefixed extensions.
> gas/
>     * testsuite/gas/riscv/march-fail-order-std.d: Removed since the relaxed
>     order checking.
>     * testsuite/gas/riscv/march-fail-order-std.l: Likewise.
>     * testsuite/gas/riscv/march-fail-order-x-std.d: Likewise.
>     * testsuite/gas/riscv/march-fail-order-z-std.d: Likewise.
>     * testsuite/gas/riscv/march-fail-order-zx-std.l: Likewise.
>     * testsuite/gas/riscv/march-fail-unknown-std.l: Updated.
>     * testsuite/gas/riscv/march-ok-reorder.d: New testcase.
> ---
>  bfd/elfxx-riscv.c                             | 210 +++++++-----------
>  .../gas/riscv/march-fail-order-std.d          |   3 -
>  .../gas/riscv/march-fail-order-std.l          |   2 -
>  .../gas/riscv/march-fail-order-x-std.d        |   3 -
>  .../gas/riscv/march-fail-order-z-std.d        |   3 -
>  .../gas/riscv/march-fail-order-zx-std.l       |   2 -
>  .../gas/riscv/march-fail-unknown-std.l        |   2 +-
>  gas/testsuite/gas/riscv/march-ok-reorder.d    |   7 +
>  8 files changed, 83 insertions(+), 149 deletions(-)
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-std.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-std.l
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-std.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z-std.d
>  delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-zx-std.l
>  create mode 100644 gas/testsuite/gas/riscv/march-ok-reorder.d
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 0bcf2fdcfa3..423c67ad344 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1275,7 +1275,7 @@ enum riscv_prefix_ext_class
>    RV_ISA_CLASS_S,
>    RV_ISA_CLASS_ZXM,
>    RV_ISA_CLASS_X,
> -  RV_ISA_CLASS_UNKNOWN
> +  RV_ISA_CLASS_SINGLE
>  };
>
>  /* Record the strings of the prefixed extensions, and their corresponding
> @@ -1296,7 +1296,7 @@ static const struct riscv_parse_prefix_config parse_config[] =
>    {RV_ISA_CLASS_Z, "z"},
>    {RV_ISA_CLASS_S, "s"},
>    {RV_ISA_CLASS_X, "x"},
> -  {RV_ISA_CLASS_UNKNOWN, NULL}
> +  {RV_ISA_CLASS_SINGLE, NULL}
>  };
>
>  /* Get the prefixed name class for the extensions, the class also
> @@ -1306,14 +1306,14 @@ static enum riscv_prefix_ext_class
>  riscv_get_prefix_class (const char *arch)
>  {
>    int i = 0;
> -  while (parse_config[i].class != RV_ISA_CLASS_UNKNOWN)
> +  while (parse_config[i].class != RV_ISA_CLASS_SINGLE)
>      {
>        if (strncmp (arch, parse_config[i].prefix,
>                    strlen (parse_config[i].prefix)) == 0)
>         return parse_config[i].class;
>        i++;
>      }
> -  return RV_ISA_CLASS_UNKNOWN;
> +  return RV_ISA_CLASS_SINGLE;
>  }
>
>  /* Check KNOWN_EXTS to see if the EXT is supported.  */
> @@ -1405,9 +1405,9 @@ riscv_compare_subsets (const char *subset1, const char *subset2)
>    enum riscv_prefix_ext_class class1 = riscv_get_prefix_class (subset1);
>    enum riscv_prefix_ext_class class2 = riscv_get_prefix_class (subset2);
>
> -  if (class1 != RV_ISA_CLASS_UNKNOWN)
> +  if (class1 != RV_ISA_CLASS_SINGLE)
>      order1 = - (int) class1;
> -  if (class2 != RV_ISA_CLASS_UNKNOWN)
> +  if (class2 != RV_ISA_CLASS_SINGLE)
>      order2 = - (int) class2;
>
>    if (order1 == order2)
> @@ -1659,7 +1659,7 @@ riscv_parsing_subset_version (const char *p,
>    return p;
>  }
>
> -/* Parsing function for standard extensions.
> +/* Parsing function for both standard and prefixed extensions.
>
>     Return Value:
>       Points to the end of extensions.
> @@ -1670,9 +1670,9 @@ riscv_parsing_subset_version (const char *p,
>       `p`: Curent parsing position.  */
>
>  static const char *
> -riscv_parse_std_ext (riscv_parse_subset_t *rps,
> -                    const char *arch,
> -                    const char *p)
> +riscv_parse_extensions (riscv_parse_subset_t *rps,
> +                       const char *arch,
> +                       const char *p)
>  {
>    /* First letter must start with i, e or g.  */
>    if (*p != 'e' && *p != 'i' && *p != 'g')
> @@ -1683,79 +1683,7 @@ riscv_parse_std_ext (riscv_parse_subset_t *rps,
>        return NULL;
>      }
>
> -  while (p != NULL && *p != '\0')
> -    {
> -      /* Stop when we parsed the known prefix class.  */
> -      enum riscv_prefix_ext_class class = riscv_get_prefix_class (p);
> -      if (class != RV_ISA_CLASS_UNKNOWN)
> -       break;
> -
> -      if (*p == '_')
> -       {
> -         p++;
> -         continue;
> -       }
> -
> -      bool implicit = false;
> -      int major = RISCV_UNKNOWN_VERSION;
> -      int minor = RISCV_UNKNOWN_VERSION;
> -      char subset[2] = {0, 0};
> -
> -      subset[0] = *p;
> -
> -      /* Check if the standard extension is supported.  */
> -      if (riscv_ext_order[(subset[0] - 'a')] == 0)
> -       {
> -         rps->error_handler
> -           (_("%s: unknown standard ISA extension `%c'"),
> -            arch, subset[0]);
> -         return NULL;
> -       }
> -
> -      /* Checking canonical order.  */
> -      if (rps->subset_list->tail != NULL
> -         && riscv_compare_subsets (rps->subset_list->tail->name, subset) > 0)
> -       {
> -         rps->error_handler
> -           (_("%s: standard ISA extension `%c' is not "
> -              "in canonical order"), arch, subset[0]);
> -         return NULL;
> -       }
> -
> -      p = riscv_parsing_subset_version (++p, &major, &minor);
> -      /* Added g as an implicit extension.  */
> -      if (subset[0] == 'g')
> -       {
> -         implicit = true;
> -         major = RISCV_UNKNOWN_VERSION;
> -         minor = RISCV_UNKNOWN_VERSION;
> -       }
> -      riscv_parse_add_subset (rps, subset, major, minor, implicit);
> -    }
> -
> -  return p;
> -}
> -
> -/* Parsing function for prefixed extensions.
> -
> -   Return Value:
> -     Points to the end of extension.
> -
> -   Arguments:
> -     `rps`: Hooks and status for parsing extensions.
> -     `arch`: Full ISA string.
> -     `p`: Curent parsing position.  */
> -
> -static const char *
> -riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
> -                         const char *arch,
> -                         const char *p)
> -{
> -  int major_version;
> -  int minor_version;
> -  enum riscv_prefix_ext_class class;
> -
> -  while (*p)
> +  while (*p != '\0')
>      {
>        if (*p == '_')
>         {
> @@ -1763,52 +1691,62 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>           continue;
>         }
>
> -      class = riscv_get_prefix_class (p);
> -      if (class == RV_ISA_CLASS_UNKNOWN)
> -       {
> -         rps->error_handler
> -           (_("%s: unknown prefix class for the ISA extension `%s'"),
> -            arch, p);
> -         return NULL;
> -       }
> -
>        char *subset = xstrdup (p);
> -      char *q = subset;
> +      char *q = subset;        /* Start of version.  */
>        const char *end_of_version;
> +      bool implicit = false;
>
> -      /* Extract the whole prefixed extension by '_'.  */
> -      while (*++q != '\0' && *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)
> +      enum riscv_prefix_ext_class class = riscv_get_prefix_class (p);
> +      if (class == RV_ISA_CLASS_SINGLE)
>         {
> -         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;
> +         if (riscv_ext_order[(*subset - 'a')] == 0)
> +           {
> +             rps->error_handler
> +               (_("%s: unknown standard ISA extension or prefix class `%c'"),
> +                arch, *subset);
> +             free (subset);
> +             return NULL;
> +           }
> +         q++;
>         }
> -      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)))
> +      else
>         {
> -         *q = '\0';
> -         rps->error_handler
> -           (_("%s: invalid prefixed ISA extension `%s' ends with <number>p"),
> -            arch, subset);
> -         free (subset);
> -         return NULL;
> +         /* Extract the whole prefixed extension by '_'.  */
> +         while (*++q != '\0' && *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
> +               (_("%s: invalid prefixed ISA extension `%s' ends with <number>p"),
> +                arch, subset);
> +             free (subset);
> +             return NULL;
> +           }
>         }
>
> +      int major_version = RISCV_UNKNOWN_VERSION;
> +      int minor_version = RISCV_UNKNOWN_VERSION;
>        end_of_version =
>         riscv_parsing_subset_version (q, &major_version, &minor_version);
>        *q = '\0';
> @@ -1818,8 +1756,9 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>           return NULL;
>         }
>
> -      /* Check that the extension name is well-formed.  */
> -      if (rps->check_unknown_prefixed_ext
> +      /* Check if the prefixed extension name is well-formed.  */
> +      if (class != RV_ISA_CLASS_SINGLE
> +         && rps->check_unknown_prefixed_ext
>           && !riscv_recognized_prefixed_ext (subset))
>         {
>           rps->error_handler
> @@ -1829,13 +1768,22 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps,
>           return NULL;
>         }
>
> +      /* Added g as an implicit extension.  */
> +      if (class == RV_ISA_CLASS_SINGLE
> +         && strcmp (subset, "g") == 0)
> +       {
> +         implicit = true;
> +         major_version = RISCV_UNKNOWN_VERSION;
> +         minor_version = RISCV_UNKNOWN_VERSION;
> +       }
>        riscv_parse_add_subset (rps, subset,
>                               major_version,
> -                             minor_version, false);
> +                             minor_version, implicit);
>        p += end_of_version - subset;
>        free (subset);
>
> -      if (*p != '\0' && *p != '_')
> +      if (class != RV_ISA_CLASS_SINGLE
> +         && *p != '\0' && *p != '_')
>         {
>           rps->error_handler
>             (_("%s: prefixed ISA extension must separate with _"),
> @@ -2008,16 +1956,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
>        return false;
>      }
>
> -  /* Parsing standard extension.  */
> -  p = riscv_parse_std_ext (rps, arch, p);
> -
> -  if (p == NULL)
> -    return false;
> -
> -  /* Parse prefixed extensions.  */
> -  p = riscv_parse_prefixed_ext (rps, arch, p);
> -
> -  if (p == NULL)
> +  /* Parse single standard and prefixed extensions.  */
> +  if (riscv_parse_extensions (rps, arch, p) == NULL)
>      return false;
>
>    /* Finally add implicit extensions according to the current
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-std.d b/gas/testsuite/gas/riscv/march-fail-order-std.d
> deleted file mode 100644
> index b9c7e09de1a..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-std.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32iamfd
> -#source: empty.s
> -#error_output: march-fail-order-std.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-std.l b/gas/testsuite/gas/riscv/march-fail-order-std.l
> deleted file mode 100644
> index 9e3ce5e8d91..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-std.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*standard ISA extension `m' is not in canonical order
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-std.d b/gas/testsuite/gas/riscv/march-fail-order-x-std.d
> deleted file mode 100644
> index 4762f3dd95c..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-x-std.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_xargle2p0_mafd
> -#source: empty.s
> -#error_output: march-fail-order-zx-std.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-z-std.d b/gas/testsuite/gas/riscv/march-fail-order-z-std.d
> deleted file mode 100644
> index 42526de3621..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-z-std.d
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#as: -march=rv32i_zicsr2p0_mafd
> -#source: empty.s
> -#error_output: march-fail-order-zx-std.l
> diff --git a/gas/testsuite/gas/riscv/march-fail-order-zx-std.l b/gas/testsuite/gas/riscv/march-fail-order-zx-std.l
> deleted file mode 100644
> index 4f6b98c592e..00000000000
> --- a/gas/testsuite/gas/riscv/march-fail-order-zx-std.l
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -.*Assembler messages:
> -.*Error: .*unknown prefix class for the ISA extension `mafd'
> diff --git a/gas/testsuite/gas/riscv/march-fail-unknown-std.l b/gas/testsuite/gas/riscv/march-fail-unknown-std.l
> index 834a4857c58..1de7a420e9b 100644
> --- a/gas/testsuite/gas/riscv/march-fail-unknown-std.l
> +++ b/gas/testsuite/gas/riscv/march-fail-unknown-std.l
> @@ -1,2 +1,2 @@
>  .*Assembler messages:
> -.*Error: .*unknown standard ISA extension `y'
> +.*Error: .*unknown standard ISA extension or prefix class `y'
> diff --git a/gas/testsuite/gas/riscv/march-ok-reorder.d b/gas/testsuite/gas/riscv/march-ok-reorder.d
> new file mode 100644
> index 00000000000..030f8b15018
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/march-ok-reorder.d
> @@ -0,0 +1,7 @@
> +#as: -misa-spec=20191213 -march=rv32i2azicsr_fc2p0dxfoo2p0_m1_xbar2p0_zba
> +#source: empty.s
> +#readelf: -A
> +
> +Attribute Section: riscv
> +File Attributes
> +  Tag_RISCV_arch: "rv32i2p0_m1p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zmmul1p0_zba1p0_xbar2p0_xfoo2p0"
> --
> 2.37.1 (Apple Git-137.1)
>


More information about the Binutils mailing list