[RFC] RISC-V: Support RISC-V Profiles in -march option.
Checks
Commit Message
Supports RISC-V profiles[1] in -march option.
Default input set the profile is before other formal extensions.
[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-29.c: New test.
* gcc.target/riscv/arch-30.c: New test.
* gcc.target/riscv/arch-31.c: New test.
---
gcc/common/config/riscv/riscv-common.cc | 58 +++++++++++++++++++++++-
gcc/config/riscv/riscv-subset.h | 2 +
gcc/testsuite/gcc.target/riscv/arch-29.c | 5 ++
gcc/testsuite/gcc.target/riscv/arch-30.c | 5 ++
gcc/testsuite/gcc.target/riscv/arch-31.c | 5 ++
6 files changed, 81 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/arch-29.c
create mode 100644 gcc/testsuite/gcc.target/riscv/arch-30.c
create mode 100644 gcc/testsuite/gcc.target/riscv/arch-31.c
Comments
On 11/20/23 12:14, Jiawei wrote:
> Supports RISC-V profiles[1] in -march option.
>
> Default input set the profile is before other formal extensions.
>
> [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-29.c: New test.
> * gcc.target/riscv/arch-30.c: New test.
> * gcc.target/riscv/arch-31.c: New test.
>
> ---
> gcc/common/config/riscv/riscv-common.cc | 58 +++++++++++++++++++++++-
> gcc/config/riscv/riscv-subset.h | 2 +
> gcc/testsuite/gcc.target/riscv/arch-29.c | 5 ++
> gcc/testsuite/gcc.target/riscv/arch-30.c | 5 ++
> gcc/testsuite/gcc.target/riscv/arch-31.c | 5 ++
> 6 files changed, 81 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-29.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-30.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-31.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index 5111626157b..30617e619b1 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -165,6 +165,12 @@ struct riscv_ext_version
> int minor_version;
> };
>
> +struct riscv_profiles
> +{
> + const char * profile_name;
> + const char * profile_string;
> +};
Just a formatting nit, no space between the '*' and the field name.
> @@ -348,6 +354,28 @@ static const struct riscv_ext_version riscv_combine_info[] =
> {NULL, ISA_SPEC_CLASS_NONE, 0, 0}
> };
>
> +static const riscv_profiles riscv_profiles_table[] =
> +{
> + {"RVI20U64", "rv64i"},
> + {"RVI20U32", "rv32i"},
> + /*Currently we don't have zicntr,ziccif,ziccrse,ziccamoa,
> + zicclsm,za128rs yet. */
It is actually useful to note the extensions not included? I don't
think the profiles are supposed to change once ratified.
> + {"RVA22U64", "rv64imafdc_zicsr_zihintpause_zba_zbb_zbs_" \
Note the trailing "_", was that intentional? None of the other entries
have a trailing "_".
> @@ -927,6 +955,31 @@ riscv_subset_list::parsing_subset_version (const char *ext,
> return p;
> }
>
> +const char *
> +riscv_subset_list::parse_profiles (const char * p){
> + 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, concatenate profiles with other ext. */
> + else {
> + size_t arch_len = strlen(riscv_profiles_table[i].profile_string) +
> + strlen(plus_ext);
> + 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;
> +}
This needs a function comment.
The open curly should always be on a line by itself which is going to
require reindenting all this code. Comments go on separate lines rather
than appending them to an existing line.
I think the consensus in the Tuesday patchwork meeting was that while
there are concerns about profiles, those concerns should prevent this
patch from going forward. So if you could fix the formatting problem as
well as the trailing "_" issue noted above and repost, it would be
appreciated.
Thanks,
Jeff
> -----原始邮件-----
> 发件人: "Jeff Law" <jeffreyalaw@gmail.com>
> 发送时间: 2023-12-12 00:15:44 (星期二)
> 收件人: Jiawei <jiawei@iscas.ac.cn>, gcc-patches@gcc.gnu.org
> 抄送: kito.cheng@sifive.com, palmer@dabbelt.com, christoph.muellner@vrull.eu
> 主题: Re: [RFC] RISC-V: Support RISC-V Profiles in -march option.
>
>
>
> On 11/20/23 12:14, Jiawei wrote:
> > Supports RISC-V profiles[1] in -march option.
> >
> > Default input set the profile is before other formal extensions.
> >
> > [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-29.c: New test.
> > * gcc.target/riscv/arch-30.c: New test.
> > * gcc.target/riscv/arch-31.c: New test.
> >
> > ---
> > gcc/common/config/riscv/riscv-common.cc | 58 +++++++++++++++++++++++-
> > gcc/config/riscv/riscv-subset.h | 2 +
> > gcc/testsuite/gcc.target/riscv/arch-29.c | 5 ++
> > gcc/testsuite/gcc.target/riscv/arch-30.c | 5 ++
> > gcc/testsuite/gcc.target/riscv/arch-31.c | 5 ++
> > 6 files changed, 81 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-29.c
> > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-30.c
> > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-31.c
> >
> > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> > index 5111626157b..30617e619b1 100644
> > --- a/gcc/common/config/riscv/riscv-common.cc
> > +++ b/gcc/common/config/riscv/riscv-common.cc
> > @@ -165,6 +165,12 @@ struct riscv_ext_version
> > int minor_version;
> > };
> >
> > +struct riscv_profiles
> > +{
> > + const char * profile_name;
> > + const char * profile_string;
> > +};
> Just a formatting nit, no space between the '*' and the field name.
Fixed.
>
> > @@ -348,6 +354,28 @@ static const struct riscv_ext_version riscv_combine_info[] =
> > {NULL, ISA_SPEC_CLASS_NONE, 0, 0}
> > };
> >
> > +static const riscv_profiles riscv_profiles_table[] =
> > +{
> > + {"RVI20U64", "rv64i"},
> > + {"RVI20U32", "rv32i"},
> > + /*Currently we don't have zicntr,ziccif,ziccrse,ziccamoa,
> > + zicclsm,za128rs yet. */
> It is actually useful to note the extensions not included? I don't
> think the profiles are supposed to change once ratified.
>
> > + {"RVA22U64", "rv64imafdc_zicsr_zihintpause_zba_zbb_zbs_" \
> Note the trailing "_", was that intentional? None of the other entries
> have a trailing "_".
Here is a line break due to too long length of arch string,
Adjusted the format in the new patch.
>
>
> > @@ -927,6 +955,31 @@ riscv_subset_list::parsing_subset_version (const char *ext,
> > return p;
> > }
> >
> > +const char *
> > +riscv_subset_list::parse_profiles (const char * p){
> > + 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, concatenate profiles with other ext. */
> > + else {
> > + size_t arch_len = strlen(riscv_profiles_table[i].profile_string) +
> > + strlen(plus_ext);
> > + 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;
> > +}
> This needs a function comment.
Thanks, added the parse function descrption and some deal logical.
>
> The open curly should always be on a line by itself which is going to
> require reindenting all this code. Comments go on separate lines rather
> than appending them to an existing line.
>
>
> I think the consensus in the Tuesday patchwork meeting was that while
> there are concerns about profiles, those concerns should prevent this
> patch from going forward. So if you could fix the formatting problem as
> well as the trailing "_" issue noted above and repost, it would be
> appreciated.
>
> Thanks,
>
> Jeff
Thanks for your review and comments, I had update them in the new patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640324.html
BR,
Jiawei</jiawei@iscas.ac.cn></jeffreyalaw@gmail.com>
@@ -165,6 +165,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[] =
{
@@ -348,6 +354,28 @@ static const struct riscv_ext_version riscv_combine_info[] =
{NULL, ISA_SPEC_CLASS_NONE, 0, 0}
};
+static const riscv_profiles riscv_profiles_table[] =
+{
+ {"RVI20U64", "rv64i"},
+ {"RVI20U32", "rv32i"},
+ /*Currently we don't have zicntr,ziccif,ziccrse,ziccamoa,
+ zicclsm,za128rs yet. */
+ {"RVA20U64", "rv64imafdc_zicsr"},
+ /*Ss1p11, svbare, sv39, svade, sscptr, ssvecd, sstvala should
+ control by binutils. */
+ {"RVA20S64", "rv64imafdc_zicsr_zifencei"},
+ /*Currently we don't have zicntr,zihpm,ziccif,ziccrse,ziccamoa,
+ zicclsm,zic64b,za64rs yet. */
+ {"RVA22U64", "rv64imafdc_zicsr_zihintpause_zba_zbb_zbs_" \
+ "zicbom_zicbop_zicboz_zfhmin_zkt"},
+ /*Ss1p12, svbare, sv39, svade, sscptr, ssvecd, sstvala,
+ scounterenw should control by binutils. */
+ {"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) \
@@ -927,6 +955,31 @@ riscv_subset_list::parsing_subset_version (const char *ext,
return p;
}
+const char *
+riscv_subset_list::parse_profiles (const char * p){
+ 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, concatenate profiles with other ext. */
+ else {
+ size_t arch_len = strlen(riscv_profiles_table[i].profile_string) +
+ strlen(plus_ext);
+ 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:
@@ -1430,7 +1483,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;
@@ -1443,7 +1499,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;
}
@@ -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 ();
new file mode 100644
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-march=RVI20U64 -mabi=lp64" } */
+int foo()
+{
+}
new file mode 100644
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-march=RVI20U64+mafdc -mabi=lp64d" } */
+int foo()
+{
+}
new file mode 100644
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-march=RVA22S64 -mabi=lp64d" } */
+int foo()
+{
+}