I don't see there is conflict if we want to support both gnu2024 and RVI profiles? also I am not sure what the usage scenarios for the gnu2024 and how we defined that?
On Wed, Sep 4, 2024 at 6:49 AM Palmer Dabbelt <pal...@rivosinc.com> wrote: > > On Tue, 20 Aug 2024 23:18:36 PDT (-0700), jia...@iscas.ac.cn wrote: > > > > 在 2024/8/21 3:23, Palmer Dabbelt 写道: > >> On Mon, 19 Aug 2024 21:53:54 PDT (-0700), jia...@iscas.ac.cn wrote: > >>> Supports RISC-V profiles[1] in -march option. > >>> > >>> Default input set the profile before other formal extensions. > >>> > >>> V2: Fixes some format errors and adds code comments for parse function > >>> Thanks for Jeff Law's review and comments. > >>> > >>> V3: Update testcases and profiles extensions support.Remove S/M mode > >>> Profiles. > >>> Thanks for Christoph Müllner,Palmer Dabbelt's review and comments. > >>> > >>> V4: Fix format issue, adjust test name. > >>> > >>> [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_base_ext): New process. > >>> * config/riscv/riscv-subset.h: New protype. > >>> > >>> gcc/testsuite/ChangeLog: > >>> > >>> * gcc.target/riscv/arch-44.c: New test. > >>> * gcc.target/riscv/arch-45.c: New test. > >>> * gcc.target/riscv/arch-46.c: New test. > >>> > >>> --- > >>> gcc/common/config/riscv/riscv-common.cc | 75 +++++++++++++++++++++++- > >>> gcc/config/riscv/riscv-subset.h | 2 + > >>> gcc/testsuite/gcc.target/riscv/arch-44.c | 5 ++ > >>> gcc/testsuite/gcc.target/riscv/arch-45.c | 12 ++++ > >>> gcc/testsuite/gcc.target/riscv/arch-46.c | 12 ++++ > >>> 5 files changed, 105 insertions(+), 1 deletion(-) > >>> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-44.c > >>> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-45.c > >>> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-46.c > >>> > >>> diff --git a/gcc/common/config/riscv/riscv-common.cc > >>> b/gcc/common/config/riscv/riscv-common.cc > >>> index 62c6e1dab1f..0bad4426971 100644 > >>> --- a/gcc/common/config/riscv/riscv-common.cc > >>> +++ b/gcc/common/config/riscv/riscv-common.cc > >>> @@ -234,6 +234,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[] = > >>> { > >>> @@ -449,6 +455,31 @@ 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 extension 'i' as mandatory > >>> extension. */ > >>> + {"RVI20U64", "rv64i"}, > >>> + {"RVI20U32", "rv32i"}, > >>> + > >>> + /* RVA20U contains the > >>> 'i,m,a,f,d,c,zicsr,zicntr,ziccif,ziccrse,ziccamoa, > >>> + zicclsm,za128rs' as mandatory extensions. */ > >>> + {"RVA20U64", "rv64imafdc_zicsr_zicntr_ziccif_ziccrse_ziccamoa" > >>> + "_zicclsm_za128rs"}, > >>> + > >>> + /* RVA22U contains the > >>> 'i,m,a,f,d,c,zicsr,zihintpause,zba,zbb,zbs,zicntr, > >>> + zihpm,ziccif,ziccrse,ziccamoa, > >>> zicclsm,zic64b,za64rs,zicbom,zicbop,zicboz, > >> > >> Except at least the Spacemit stuff that claims RVA22 doesn't actually > >> have Zicclsm, at least assuming the "supports" in there means "doesn't > >> trap" (we could just say "supports" includes traps, and thus Zicclsm > >> means nothing). > >> > >> I'd argue we should just punt on the profiles until we figure out what > >> they're actually going to be. The pre-23 profiles were all minor > >> releases anyway, so it's not like we should be losing much there (as > >> they're not meant for software). At least if we wait we don't end up > >> committing to this whole "profiles don't mean anything" spot we're in, > >> like we did for the other spec flavors. > >> > >> Though now that I'm writing that it actually just sounds kind of silly > >> to keep hoping that we're going to get any meaningful compatibility > >> rules enforced by the RISC-V foundation. There's really just no > >> incentive for that to happen, as we keep bailing out vendors who ship > >> broken systems and thus there's no pushback from their members. > >> > >> So maybe the right answer here is to just break users and tell them to > >> go complain to someone else? At least that way everyone will be > >> upset, maybe that'll be enough to get things to change? > > > > Okay, let's continue to wait for the RVA/B23 forzen. > > I actually don't think that's going to change anything. The problem > here is really enforcing the compatibility rules, and I don't see how > another round of profiles is going to change that. We're already > starting to see the backpedalling start again with the A/B and the > renaming, there's been so many rounds of this it's getting pretty > predictable. > > It's really more a political thing than a technical thing, and with the > hardware vendors being in charge of things at the RISC-V foundation > there's just no incentive to enforce compatibility. That'd just lead to > them needing to re-spin broken chips. I don't see how that changes > until we get enough successful RISC-V based products that the > fragmentation gets expensive, but that's going to take a while. > > IMO we should just give up on getting compatibility rules from the > RISC-V foundation and just build our own. That's basically what we've > done in Linux land and it's working great -- it doesn't really change > the fundamentals, it just cuts out a bunch of word games about what the > specs mean. > > So essentially just have a "-march=gnu2024" type argument, and define > that to mean everything that's shipped in 2024 that's interesting for > binary-compatible distributions. > > >>> + zfhmin,zkt' as mandatory extensions. */ > >>> + {"RVA22U64", "rv64imafdc_zicsr_zicntr_ziccif_ziccrse_ziccamoa" > >>> + "_zicclsm_zic64b_za64rs_zihintpause_zba_zbb_zbs_zicbom_zicbop" > >>> + "_zicboz_zfhmin_zkt"}, > >>> + > >>> + /* Currently we do not define S/M mode Profiles in gcc part. */ > >>> + > >>> + /* Terminate the list. */ > >>> + {NULL, NULL} > >>> +}; > >>> + > >>> static const riscv_cpu_info riscv_cpu_tables[] = > >>> { > >>> #define RISCV_CORE(CORE_NAME, ARCH, TUNE) \ > >>> @@ -1056,6 +1087,46 @@ 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 Profiles in -march option: > >>> + > >>> + 1. Only use Profiles 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. */ > >>> + 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 base extensions, rv[32|64][i|e|g] > >>> > >>> Return Value: > >>> @@ -1070,6 +1141,8 @@ riscv_subset_list::parse_base_ext (const char *p) > >>> unsigned minor_version = 0; > >>> bool explicit_version_p = false; > >>> > >>> + p = parse_profiles(p); > >>> + > >>> if (startswith (p, "rv32")) > >>> { > >>> m_xlen = 32; > >>> @@ -1082,7 +1155,7 @@ riscv_subset_list::parse_base_ext (const char *p) > >>> } > >>> else > >>> { > >>> - error_at (m_loc, "%<-march=%s%>: ISA string must begin with > >>> rv32 or rv64", > >>> + error_at (m_loc, "%<-march=%s%>: ISA string must begin with > >>> rv32, rv64 or Profile", > >>> m_arch); > >>> return NULL; > >>> } > >>> diff --git a/gcc/config/riscv/riscv-subset.h > >>> b/gcc/config/riscv/riscv-subset.h > >>> index dace4de6575..98fd9877f74 100644 > >>> --- a/gcc/config/riscv/riscv-subset.h > >>> +++ b/gcc/config/riscv/riscv-subset.h > >>> @@ -80,6 +80,8 @@ private: > >>> const char *parse_single_multiletter_ext (const char *, const char *, > >>> const char *, bool); > >>> > >>> + 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-44.c > >>> b/gcc/testsuite/gcc.target/riscv/arch-44.c > >>> new file mode 100644 > >>> index 00000000000..41190bc5939 > >>> --- /dev/null > >>> +++ b/gcc/testsuite/gcc.target/riscv/arch-44.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-45.c > >>> b/gcc/testsuite/gcc.target/riscv/arch-45.c > >>> new file mode 100644 > >>> index 00000000000..273c6abf60d > >>> --- /dev/null > >>> +++ b/gcc/testsuite/gcc.target/riscv/arch-45.c > >>> @@ -0,0 +1,12 @@ > >>> +/* { dg-do compile } */ > >>> +/* { dg-options "-march=RVI20U64+mafdc -mabi=lp64d" } */ > >>> +#if !(defined __riscv_mul) || \ > >>> + !(defined __riscv_atomic) || \ > >>> + !(defined __riscv_flen) || \ > >>> + !(defined __riscv_div) || \ > >>> + !(defined __riscv_compressed) > >>> +#error "Feature macros not defined" > >>> +#endif > >>> +int > >>> +foo () > >>> +{} > >>> diff --git a/gcc/testsuite/gcc.target/riscv/arch-46.c > >>> b/gcc/testsuite/gcc.target/riscv/arch-46.c > >>> new file mode 100644 > >>> index 00000000000..1ebf50d3755 > >>> --- /dev/null > >>> +++ b/gcc/testsuite/gcc.target/riscv/arch-46.c > >>> @@ -0,0 +1,12 @@ > >>> +/* { dg-do compile } */ > >>> +/* { dg-options "-march=RVA20U64 -mabi=lp64d" } */ > >>> +#if !(defined __riscv_mul) || \ > >>> + !(defined __riscv_atomic) || \ > >>> + !(defined __riscv_flen) || \ > >>> + !(defined __riscv_div) || \ > >>> + !(defined __riscv_compressed) > >>> +#error "Feature macros not defined" > >>> +#endif > >>> +int > >>> +foo () > >>> +{}