On Tue, Dec 12, 2023 at 1:08 PM Jiawei <jia...@iscas.ac.cn> wrote:
>
> Supports RISC-V profiles[1] in -march option.
>
> Default input set the profile is before other formal extensions.
>
> V2: Fixes some format errors and adds code comments for parse function
> Thanks for Jeff Law's review and comments.
>
> [1]https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc (struct riscv_profiles):
> New struct.
> (riscv_subset_list::parse_profiles): New function.
> (riscv_subset_list::parse): New table.
> * config/riscv/riscv-subset.h: New protype.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/arch-31.c: New test.
> * gcc.target/riscv/arch-32.c: New test.
> * gcc.target/riscv/arch-33.c: New test.
> * gcc.target/riscv/arch-34.c: New test.
For the positive tests (-31.c and -33.c) it would be great to test if
the enabled extension's test macros are set.
Something like this would do:
#if (!(defined __riscv_zicsr) || \
!(defined __riscv_...))
#error "Feature macros not defined"
#endif
Also, positive tests for RVI20U32 and RVI20U64 would be nice.
>
> ---
> gcc/common/config/riscv/riscv-common.cc | 83 +++++++++++++++++++++++-
> gcc/config/riscv/riscv-subset.h | 2 +
> gcc/testsuite/gcc.target/riscv/arch-31.c | 5 ++
> gcc/testsuite/gcc.target/riscv/arch-32.c | 5 ++
> gcc/testsuite/gcc.target/riscv/arch-33.c | 5 ++
> gcc/testsuite/gcc.target/riscv/arch-34.c | 7 ++
> 6 files changed, 106 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-31.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-32.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-33.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-34.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc
> b/gcc/common/config/riscv/riscv-common.cc
> index 4d5a2f874a2..8b674a4a280 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -195,6 +195,12 @@ struct riscv_ext_version
> int minor_version;
> };
>
> +struct riscv_profiles
> +{
> + const char *profile_name;
> + const char *profile_string;
> +};
> +
> /* All standard extensions defined in all supported ISA spec. */
> static const struct riscv_ext_version riscv_ext_version_table[] =
> {
> @@ -379,6 +385,42 @@ static const struct riscv_ext_version
> riscv_combine_info[] =
> {NULL, ISA_SPEC_CLASS_NONE, 0, 0}
> };
>
> +/* This table records the mapping form RISC-V Profiles into march string. */
> +static const riscv_profiles riscv_profiles_table[] =
> +{
> + /* RVI20U only contains the base extesnion 'i' as mandatory extension. */
> + {"RVI20U64", "rv64i"},
> + {"RVI20U32", "rv32i"},
> +
> + /* RVA20U contains the 'i,m,a,f,d,c,zicsr' as mandatory extensions.
> + Currently we don't have zicntr,ziccif,ziccrse,ziccamoa,
> + zicclsm,za128rs yet. */
> + {"RVA20U64", "rv64imafdc_zicsr"},
> +
> + /* RVA20S64 mandatory include all the extensions in RVA20U64 and
> + additonal 'zifencei' as mandatory extensions.
> + Notes that ss1p11, svbare, sv39, svade, sscptr, ssvecd, sstvala should
> + control by binutils. */
> + {"RVA20S64", "rv64imafdc_zicsr_zifencei"},
> +
> + /* RVA22U contains the 'i,m,a,f,d,c,zicsr,zihintpause,zba,zbb,zbs,
> + zicbom,zicbop,zicboz,zfhmin,zkt' as mandatory extensions.
> + Currently we don't have zicntr,zihpm,ziccif,ziccrse,ziccamoa,
> + zicclsm,zic64b,za64rs yet. */
I would prefer that we implement the missing extensions that start
with 'z' as "dummy" extensions.
I.e., they (currently?) don't affect code generation, but they will be
passed on to the assembler and
will become part of the Tag_RISCV_arch string.
I admit that such "dummy" extensions may not be preferred by
maintainers, but we already
have precedence with Zkt.
I consider an incomplete expansion of a profile as misleading.
And later changes to complete the expansion could be called out as
"breaking changes".
> + {"RVA22U64", "rv64imafdc_zicsr_zihintpause_zba_zbb_zbs"
> \
> + "_zicbom_zicbop_zicboz_zfhmin_zkt"},
> +
> + /* RVA22S64 mandatory include all the extensions in RVA22U64 and
> + additonal 'zifencei,svpbmt,svinval' as mandatory extensions.
> + Notes that ss1p12, svbare, sv39, svade, sscptr, ssvecd, sstvala,
> + scounterenw extentions should control by binutils. */
Typo: extentions -> extensions
I want to challenge the implementation of RVA22S64 support
(or in general all S-mode and M-mode profile support) in toolchains:
* Adding 's*'/'m*' extensions as dummy extensions won't have much use
* Having an incomplete extension is misleading (see above)
* I doubt that RVA22S64 would find many users
Therefore, I would not add support for S-mode and M-mode profiles.
> + {"RVA22S64","rv64imafdc_zicsr_zifencei_zihintpause"
> \
> + "_zba_zbb_zbs_zicbom_zicbop_zicboz_zfhmin_zkt_svpbmt_svinval"},
> +
> + /* Terminate the list. */
> + {NULL, NULL}
> +};
> +
> static const riscv_cpu_info riscv_cpu_tables[] =
> {
> #define RISCV_CORE(CORE_NAME, ARCH, TUNE) \
> @@ -958,6 +1000,42 @@ riscv_subset_list::parsing_subset_version (const char
> *ext,
> return p;
> }
>
> +/* Parsing RISC-V Profiles in -march string.
> + Return string with mandatory extensions of Profiles. */
> +const char *
> +riscv_subset_list::parse_profiles (const char * p){
> + /* Checking if input string contains a Profiles.
> + There are two cases use Proifles in -march option
> +
> + 1. Only use Proifles as -march input
> + 2. Mixed Profiles with other extensions
> +
> + use '+' to split Profiles and other extension. */
> + 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 begin. */
> + if (match != NULL && match == p) {
> + /* If there's no '+' sign, return the profile_string directly. */
> + if(!plus_ext)
> + return riscv_profiles_table[i].profile_string;
> + /* If there's a '+' sign, need to add profiles with other ext. */
> + else {
> + 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. */
> + static char* result = new char[arch_len + 2];
> + strcpy(result, riscv_profiles_table[i].profile_string);
> + strcat(result, "_");
> + strcat(result, plus_ext + 1); /* skip the '+'. */
> + return result;
> + }
> + }
> + }
> + return p;
> +}
> +
> /* Parsing function for standard extensions.
>
> Return Value:
> @@ -1486,7 +1564,10 @@ riscv_subset_list::parse (const char *arch, location_t
> loc)
>
> riscv_subset_list *subset_list = new riscv_subset_list (arch, loc);
> riscv_subset_t *itr;
> +
> const char *p = arch;
> + p = subset_list->parse_profiles(p);
> +
> if (startswith (p, "rv32"))
> {
> subset_list->m_xlen = 32;
> @@ -1499,7 +1580,7 @@ riscv_subset_list::parse (const char *arch, location_t
> loc)
> }
> else
> {
> - error_at (loc, "%<-march=%s%>: ISA string must begin with rv32 or
> rv64",
> + error_at (loc, "%<-march=%s%>: ISA string must begin with rv32, rv64
> or a profile",
> arch);
> goto fail;
> }
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index ad1cab2aa24..ba991f67014 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -76,6 +76,8 @@ private:
> const char *parse_single_multiletter_ext (const char *, const char *,
> const char *);
>
> + const char *parse_profiles (const char*);
> +
> void handle_implied_ext (const char *);
> bool check_implied_ext ();
> void handle_combine_ext ();
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-31.c
> b/gcc/testsuite/gcc.target/riscv/arch-31.c
> new file mode 100644
> index 00000000000..eb24abe4153
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/arch-31.c
> @@ -0,0 +1,5 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=RVI20U64 -mabi=lp64" } */
> +int foo()
> +{
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-32.c
> b/gcc/testsuite/gcc.target/riscv/arch-32.c
> new file mode 100644
> index 00000000000..bc556a3e717
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/arch-32.c
> @@ -0,0 +1,5 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=RVI20U64+mafdc -mabi=lp64d" } */
> +int foo()
> +{
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-33.c
> b/gcc/testsuite/gcc.target/riscv/arch-33.c
> new file mode 100644
> index 00000000000..a7b80a5ad43
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/arch-33.c
> @@ -0,0 +1,5 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=RVA22S64 -mabi=lp64d" } */
> +int foo()
> +{
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-34.c
> b/gcc/testsuite/gcc.target/riscv/arch-34.c
> new file mode 100644
> index 00000000000..d077aba4fc2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/arch-34.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcRVA22S64 -mabi=lp64d" } */
> +int foo()
> +{
> +}
> +
> +/* { dg-error "ISA string must begin with rv32, rv64 or a profile" "" {
> target *-*-* } 0 } */
> --
> 2.25.1
>
