[v2 PATCH 1/2] RISC-V: Add support for RISC-V Profiles 20/22.
Nelson Chu
nelson@rivosinc.com
Tue Feb 18 06:49:25 GMT 2025
On Thu, Jan 23, 2025 at 1:30 PM Jiawei <jiawei@iscas.ac.cn> wrote:
> +const char *
> +riscv_handle_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)
> + {
> + const char *match = strstr (p,
> riscv_profiles_table[i].profile_name);
> + const char *plus_ext = strchr (p, '_');
> + /* Find profile at the beginning. */
> + if (match != NULL && match == p)
> + {
> + /* If only have profile, return the profile_string directly. */
> + if (*(match + strlen(riscv_profiles_table[i].profile_name)) ==
> '\0')
> + return riscv_profiles_table[i].profile_string;
> + /* If there's a '_' sign, need to add profiles with other ext.
> */
> + else if (*(match + strlen(riscv_profiles_table[i].profile_name))
> == '_')
> + {
> + size_t arch_len = (strlen
> (riscv_profiles_table[i].profile_string)
> + + strlen (plus_ext));
> + /* Reset the input string with Profiles mandatory extensions,
> + end with '_' to connect other additional extensions. */
> + char *result = (char *) malloc (arch_len + 2);
>
Well, malloc spaces need to be free, so see the caller riscv_parse_subset
...
> + strcpy (result, riscv_profiles_table[i].profile_string);
> + strcat (result, plus_ext); /* Add other extensions after
> '_'. */
> + return result;
> + }
> + else
> + {
> + rps->error_handler (
> + _("%s: should use '_' between profile and other
> extesnions"),
> + p);
> + return riscv_profiles_table[i].profile_string;
>
Return NULL for the error case?
> + }
> + }
> + }
> + return p;
> +}
> +
> /* Function for parsing ISA string.
>
> Return Value:
> @@ -2198,18 +2273,19 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
> return riscv_parse_check_conflicts (rps);
> }
>
> - for (p = arch; *p != '\0'; p++)
> + p = riscv_handle_profiles (rps, arch);
> +
> + for (const char *q = p; *q != '\0'; q++)
> {
>
... The p may point to a malloc profile string from riscv_handle_profiles,
or normal input const architecture string. The former needs free, but the
latter does not need to care about this in this function. It's hard to
know which one is it from the current code, so I think we can ...
> - if (ISUPPER (*p))
> + if (ISUPPER (*q))
> {
> rps->error_handler
> (_("%s: ISA string cannot contain uppercase letters"),
> - arch);
> + q);
> return false;
> }
> }
>
> - p = arch;
> if (startswith (p, "rv32"))
> {
> *rps->xlen = 32;
> @@ -2231,7 +2307,7 @@ riscv_parse_subset (riscv_parse_subset_t *rps,
> string is empty. */
> if (strlen (arch))
> rps->error_handler (
> - _("%s: ISA string must begin with rv32 or rv64"),
> + _("%s: ISA string must begin with rv32, rv64 or a profile"),
> arch);
> return false;
> }
>
... Here, Don't malloc new spaces to parse profile string, instead,
something probably like,
if (startswith (p, "rv32"))
{ *rps->xlen = 32; p += 4; }
else if (startswith (p, "rv64"))
{ *rps->xlen = 64; p += 4; }
else if (riscv_handle_profiles (...))
{ riscv_parse_subset (rps, profile_replaced_string);
p += length of profile name; }
The riscv_handle_profiles only compare the profile name, and then return
the corresponding str without malloc.
Also, maybe a new argument in riscv_parse_extensions (..., boolean
base_eig), that only checks the base is e/i/g for non-profile cases.
diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index 19e04adfa6a..d8bb61825fc 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -121,6 +121,9 @@ riscv_multi_subset_supports (riscv_parse_subset_t *,
> enum riscv_insn_class);
> extern const char *
> riscv_multi_subset_supports_ext (riscv_parse_subset_t *, enum
> riscv_insn_class);
>
> +extern const char *
> +riscv_handle_profiles(riscv_parse_subset_t *, const char*);
> +
>
Can this be static so that it doesn't need extern?
> extern void
> riscv_print_extensions (void);
>
> diff --git a/gas/NEWS b/gas/NEWS
> index 70f3ad40dba..f62e59bdd64 100644
> --- a/gas/NEWS
> +++ b/gas/NEWS
> @@ -34,6 +34,8 @@ Changes in 2.44:
> CORE-V, xcvbitmanip v1.0 and xcvsimd v1.0.
> SiFive, xsfvqmaccdod v1.0, xsfvqmaccqoqv1.0 and xsfvfnrclipxfqf v1.0.
>
> +* Add support for RISC-V Profiles.
> +
>
Should be 2.45.
> * Add support for 4 formats of .cfi directives register aliases for
> LoongArch,
> for example, .cfi_offset r1,8, .cfi_offset ra,8, .cfi_offset $r1,8,
> .cfi_offset $ra,8.
> diff --git a/gas/doc/as.texi b/gas/doc/as.texi
> index afe1737f4ce..ad096fc02c8 100644
> --- a/gas/doc/as.texi
> +++ b/gas/doc/as.texi
> @@ -552,7 +552,7 @@ gcc(1), ld(1), and the Info entries for
> @file{binutils} and @file{ld}.
>
> @emph{Target RISC-V options:}
> [@b{-fpic}|@b{-fPIC}|@b{-fno-pic}]
> - [@b{-march}=@var{ISA}]
> + [@b{-march}=@var{ISA/Profiles/Profiles+ISA}]
>
Maybe,
[@b{-march}=@var{ISA}|@var{Profile}[+@var{ISA}] ?
> [@b{-mabi}=@var{ABI}]
> [@b{-mlittle-endian}|@b{-mbig-endian}]
> @end ifset
> diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
> index d2e47455e7c..f5f13a976e5 100644
> --- a/gas/doc/c-riscv.texi
> +++ b/gas/doc/c-riscv.texi
> @@ -41,9 +41,10 @@ Generate position-independent code
> @item -fno-pic
> Don't generate position-independent code (default)
>
> -@cindex @samp{-march=ISA} option, RISC-V
> -@item -march=ISA
> -Select the base isa, as specified by ISA. For example -march=rv32ima.
> +@cindex @samp{-march=ISA/Profiles/Profies+ISA} option, RISC-V
> +@item -march=ISA/Profiles/Profiles+ISA
+Select the base isa, as specified by ISA or Profiles or Profies+ISA.
>
@samp{-march=[ISA|Profiles|Profies+ISA]}, I think we need regular
expression here.
Btw, "profiles" seem to allow adding multiple profiles?
> +For example -march=rv32ima -march=RVI20U64 -march=RVI20U64+d.
>
Comma to separate?
Thanks
Nelson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250218/4fe19498/attachment.htm>
More information about the Binutils
mailing list