Re: [RFC] RISC-V: Support RISC-V Profiles in -march option.

[Jeff Law <jeffreyalaw@gmail.com>]

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