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

[jiawei@iscas.ac.cn]

> -----原始邮件-----
&gt; 发件人: "Jeff Law" <jeffreyalaw@gmail.com>
&gt; 发送时间: 2023-12-12 00:15:44 (星期二)
&gt; 收件人: Jiawei <jiawei@iscas.ac.cn>, gcc-patches@gcc.gnu.org
&gt; 抄送: kito.cheng@sifive.com, palmer@dabbelt.com, christoph.muellner@vrull.eu
&gt; 主题: Re: [RFC] RISC-V: Support RISC-V Profiles in -march option.
&gt; 
&gt; 
&gt; 
&gt; On 11/20/23 12:14, Jiawei wrote:
&gt; &gt; Supports RISC-V profiles[1] in -march option.
&gt; &gt; 
&gt; &gt; Default input set the profile is before other formal extensions.
&gt; &gt; 
&gt; &gt; [1]https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc
&gt; &gt; 
&gt; &gt; gcc/ChangeLog:
&gt; &gt; 
&gt; &gt;          * common/config/riscv/riscv-common.cc (struct riscv_profiles):
&gt; &gt;            New struct.
&gt; &gt;          (riscv_subset_list::parse_profiles): New function.
&gt; &gt;          (riscv_subset_list::parse): New table.
&gt; &gt;          * config/riscv/riscv-subset.h: New protype.
&gt; &gt; 
&gt; &gt; gcc/testsuite/ChangeLog:
&gt; &gt; 
&gt; &gt;          * gcc.target/riscv/arch-29.c: New test.
&gt; &gt;          * gcc.target/riscv/arch-30.c: New test.
&gt; &gt;          * gcc.target/riscv/arch-31.c: New test.
&gt; &gt; 
&gt; &gt; ---
&gt; &gt;   gcc/common/config/riscv/riscv-common.cc  | 58 +++++++++++++++++++++++-
&gt; &gt;   gcc/config/riscv/riscv-subset.h          |  2 +
&gt; &gt;   gcc/testsuite/gcc.target/riscv/arch-29.c |  5 ++
&gt; &gt;   gcc/testsuite/gcc.target/riscv/arch-30.c |  5 ++
&gt; &gt;   gcc/testsuite/gcc.target/riscv/arch-31.c |  5 ++
&gt; &gt;   6 files changed, 81 insertions(+), 1 deletion(-)
&gt; &gt;   create mode 100644 gcc/testsuite/gcc.target/riscv/arch-29.c
&gt; &gt;   create mode 100644 gcc/testsuite/gcc.target/riscv/arch-30.c
&gt; &gt;   create mode 100644 gcc/testsuite/gcc.target/riscv/arch-31.c
&gt; &gt; 
&gt; &gt; diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
&gt; &gt; index 5111626157b..30617e619b1 100644
&gt; &gt; --- a/gcc/common/config/riscv/riscv-common.cc
&gt; &gt; +++ b/gcc/common/config/riscv/riscv-common.cc
&gt; &gt; @@ -165,6 +165,12 @@ struct riscv_ext_version
&gt; &gt;     int minor_version;
&gt; &gt;   };
&gt; &gt;   
&gt; &gt; +struct riscv_profiles
&gt; &gt; +{
&gt; &gt; +  const char * profile_name;
&gt; &gt; +  const char * profile_string;
&gt; &gt; +};
&gt; Just a formatting nit, no space between the '*' and the field name.

Fixed.

&gt; 
&gt; &gt; @@ -348,6 +354,28 @@ static const struct riscv_ext_version riscv_combine_info[] =
&gt; &gt;     {NULL, ISA_SPEC_CLASS_NONE, 0, 0}
&gt; &gt;   };
&gt; &gt;   
&gt; &gt; +static const riscv_profiles riscv_profiles_table[] =
&gt; &gt; +{
&gt; &gt; +  {"RVI20U64", "rv64i"},
&gt; &gt; +  {"RVI20U32", "rv32i"},
&gt; &gt; +  /*Currently we don't have zicntr,ziccif,ziccrse,ziccamoa,
&gt; &gt; +    zicclsm,za128rs yet.  */
&gt; It is actually useful to note the extensions not included?  I don't 
&gt; think the profiles are supposed to change once ratified.
&gt; 
&gt; &gt; +  {"RVA22U64", "rv64imafdc_zicsr_zihintpause_zba_zbb_zbs_"				\
&gt; Note the trailing "_", was that intentional?  None of the other entries 
&gt; have a trailing "_".

Here is a line break due to too long length of arch string,
Adjusted the format in the new patch.

&gt; 
&gt; 
&gt; &gt; @@ -927,6 +955,31 @@ riscv_subset_list::parsing_subset_version (const char *ext,
&gt; &gt;     return p;
&gt; &gt;   }
&gt; &gt;   
&gt; &gt; +const char *
&gt; &gt; +riscv_subset_list::parse_profiles (const char * p){
&gt; &gt; +  for (int i = 0; riscv_profiles_table[i].profile_name != NULL; ++i) {
&gt; &gt; +    const char* match = strstr(p, riscv_profiles_table[i].profile_name);
&gt; &gt; +    const char* plus_ext = strchr(p, '+');
&gt; &gt; +    /* Find profile at the begin.  */
&gt; &gt; +    if (match != NULL &amp;&amp; match == p) {
&gt; &gt; +      /* If there's no '+' sign, return the profile_string directly.  */
&gt; &gt; +      if(!plus_ext)
&gt; &gt; +	return riscv_profiles_table[i].profile_string;
&gt; &gt; +      /* If there's a '+' sign, concatenate profiles with other ext.  */
&gt; &gt; +      else {
&gt; &gt; +	size_t arch_len = strlen(riscv_profiles_table[i].profile_string) +
&gt; &gt; +		strlen(plus_ext);
&gt; &gt; +	static char* result = new char[arch_len + 2];
&gt; &gt; +	strcpy(result, riscv_profiles_table[i].profile_string);
&gt; &gt; +	strcat(result, "_");
&gt; &gt; +	strcat(result, plus_ext + 1); /* skip the '+'.  */
&gt; &gt; +	return result;
&gt; &gt; +      }
&gt; &gt; +    }
&gt; &gt; +  }
&gt; &gt; +  return p;
&gt; &gt; +}
&gt; This needs a function comment.

Thanks, added the parse function descrption and some deal logical.

&gt; 
&gt; The open curly should always be on a line by itself which is going to 
&gt; require reindenting all this code.  Comments go on separate lines rather 
&gt; than appending them to an existing line.
&gt; 
&gt; 
&gt; I think the consensus in the Tuesday patchwork meeting was that while 
&gt; there are concerns about profiles, those concerns should prevent this 
&gt; patch from going forward.  So if you could fix the formatting problem as 
&gt; well as the trailing "_" issue noted above and repost, it would be 
&gt; appreciated.
&gt; 
&gt; Thanks,
&gt; 
&gt; 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>