Nelson Chu nelson@rivosinc.com
Fri May 23 02:19:38 GMT 2025

• Previous message (by thread): [PATCH v3 1/2] RISC-V: Add support for RISC-V Profiles 20/22.
• Next message (by thread): [PATCH v3 2/2] RISC-V: Add support for RISC-V Profiles 23.
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

Thanks for keep doing this, basically looks good to me.  Committed after
passing the riscv-gnu-toolchain regressions, and with following changes.

Nelson

On Sun, May 11, 2025 at 9:38 PM Jiawei <jiawei@iscas.ac.cn> wrote:

> +static bool
> +riscv_find_profiles (riscv_parse_subset_t *rps, const char *p)
> +{
> +  /* Checking if input string contains a Profiles.
> +     There are two cases use Profiles in -march option:
> +
> +       1. Only use Profiles in '-march' as input
> +       2. Mixed Profiles with other extensions
> +
> +     Use '_' to split Profiles and other extensions.  */
> +
> +  for (int i = 0; riscv_profiles_table[i].profile_name != NULL; ++i)
> +  {
> +    /* Find profile at the begin.  */
> +    if (strncmp(p, riscv_profiles_table[i].profile_name,
> +                 strlen(riscv_profiles_table[i].profile_name)) == 0)
> +    {
> +      /* Handle the profile string.  */
> +      riscv_parse_subset(rps, riscv_profiles_table[i].profile_string);
> +      p += strlen(riscv_profiles_table[i].profile_name);
> +      /* Handle string after profiles if exists.  */
> +      if(*p != '\0')
> +      {
> +       /* If missing underline bewteen profile and other extensions,
> +          warn the user but not deal as an error.  */
> +       if(*p != '_')
> +         _bfd_error_handler
> +         (_("Warning: should use \"_\" to contact Profiles with other
> extensions"));
> +       riscv_parse_extensions (rps, p, p, true);
> +      }
> +      return true;
> +    }
> +  }
> +  /* Not found profile, return directly.  */
> +  return false;
> +}
> +
>

1. Fixed indents, needs space between if/function and '('.
2. Replace strlen with startswith, like others.
3. Also let riscv_parse_subset call riscv_parse_extensions later, tidy the
code.

So will be something like,

+static bool
+riscv_find_profiles (riscv_parse_subset_t *rps, const char **pp)
+{
+  const char *p = *pp;
+  for (int i = 0; riscv_profiles_table[i].profile_name != NULL; ++i)
+    {
+      if (startswith (p, riscv_profiles_table[i].profile_name))
+       {
+         if (*p != '\0' && *p != '_')
+           _bfd_error_handler (...)
+         *pp = p;
+         return true;
+       }
+    }
+  return false;
+}


>  /* Function for parsing ISA string.
>
>     Return Value:
> @@ -2275,36 +2344,39 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
>      }
>
>    p = arch;
> -  if (startswith (p, "rv32"))
> -    {
> -      *rps->xlen = 32;
> -      p += 4;
> -    }
> -  else if (startswith (p, "rv64"))
> -    {
> -      *rps->xlen = 64;
> -      p += 4;
> -    }
> -  else
> -    {
> -      /* ISA string shouldn't be NULL or empty here.  For linker,
> -        it might be empty when we failed to merge the ISA string
> -        in the riscv_merge_attributes.  For assembler, we might
> -        give an empty string by .attribute arch, "" or -march=.
> -        However, We have already issued the correct error message
> -        in another side, so do not issue this error when the ISA
> -        string is empty.  */
> -      if (strlen (arch))
> -       rps->error_handler (
> -         _("%s: ISA string must begin with rv32 or rv64"),
> -         arch);
> +  /* Check if using Profiles.  */
> +  if(!riscv_find_profiles(rps, arch))
> +  {
> +    if (startswith (p, "rv32"))
> +      {
> +       *rps->xlen = 32;
> +       p += 4;
> +      }
> +    else if (startswith (p, "rv64"))
> +      {
> +       *rps->xlen = 64;
> +       p += 4;
> +      }
> +    else
> +      {
> +       /* ISA string shouldn't be NULL or empty here.  For linker,
> +          it might be empty when we failed to merge the ISA string
> +          in the riscv_merge_attributes.  For assembler, we might
> +          give an empty string by .attribute arch, "" or -march=.
> +          However, We have already issued the correct error message
> +          in another side, so do not issue this error when the ISA
> +          string is empty.  */
> +       if (strlen (arch))
> +         rps->error_handler (
> +           _("%s: ISA string must begin with rv32, rv64 or Profiles"),
> +           arch);
> +       return false;
> +      }
> +
> +    /* Parse single standard and prefixed extensions.  */
> +    if (riscv_parse_extensions (rps, arch, p, false) == NULL)
>        return false;
> -    }
> -
> -  /* Parse single standard and prefixed extensions.  */
> -  if (riscv_parse_extensions (rps, arch, p) == NULL)
> -    return false;
> -
> +  }
>

Tidy the code to,

+  bool profile = false;
   p = arch;
-  if (startswith (p, "rv32"))
+  if (riscv_find_profiles (rps, &p))
+    {
+      /* Check if using Profiles.  */
+      profile = true;
+    }
+  else if (startswith (p, "rv32"))
     {
       *rps->xlen = 32;
       p += 4;
...
   /* Parse single standard and prefixed extensions.  */
-  if (riscv_parse_extensions (rps, arch, p) == NULL)
+  if (riscv_parse_extensions (rps, arch, p, profile) == NULL)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250523/8b285548/attachment-0001.htm>

---

• Previous message (by thread): [PATCH v3 1/2] RISC-V: Add support for RISC-V Profiles 20/22.
• Next message (by thread): [PATCH v3 2/2] RISC-V: Add support for RISC-V Profiles 23.
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

More information about the Binutils mailing list