[PATCH v5] RISC-V: Add support for RISC-V Profiles.

[Nelson Chu]

On Wed, Sep 4, 2024 at 11:12 AM Jiawei <jiawei@iscas.ac.cn> wrote:

>
> 在 2024/9/3 15:05, Nelson Chu 写道:
>
> Basically looks good, I am fine with the current implementation, just some
> minor issues probably need to be clarified,
>
> On Wed, Aug 7, 2024 at 11:44 PM Jiawei <jiawei@iscas.ac.cn> wrote:
>
>> Supports RISC-V profiles[1] in -march option.
>>
>> Default input set the profile before other formal extensions.
>>
>> [1]https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc
>
>
> 404 link needs to be updated :P
>
> Thanks for you remark, will use github repo to replace it
>
> https://github.com/riscv/riscv-profiles
>
>
>
>> +/* This table records the mapping form RISC-V Profiles into march
>> string.  */
>> +static struct 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,
>> +     zfhmin,zkt' as mandatory extensions.  */
>> +  {"RVA22U64", "rv64imafdc_zicsr_zicntr_ziccif_ziccrse_ziccamoa"
>> +   "_zicclsm_zic64b_za64rs_zihintpause_zba_zbb_zbs_zicbom_zicbop"
>> +   "_zicboz_zfhmin_zkt"},
>>
>
> According to the commit e8d4709e, RISC-V: Don't allow any uppercase letter
> in the arch string - Do the profile strings also need to be lower cases
> since they still belong to the -march option?  For example,
> -march=rvi20u64+fdc
>
> The original intention of using uppercase letters is to be consistent with
> the definition of profile naming convention in the specification manual.
> However, llvm upstream used the lowercase letters, perhaps maintaining user
> usage habits is a good choice.
>
>
>
>>
>> @@ -2148,6 +2179,45 @@ riscv_set_default_arch (riscv_parse_subset_t *rps)
>>      }
>>  }
>>
>> +const char *
>> +riscv_handle_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 extensions.  */
>>
>
> Not sure if using `+' as the separator is a good idea, since its usage
> seems confused with the `+' in .option arch.  I think maybe we can clarify
> the architecture string that must be started with, rv32, rv64, or defined
> profile string.  So that the other extensions can simply be attached after
> `_'.  That is - startswith (arch, string), string can be "rv32", "rv64", or
> profile strings "rvi20u64", ....
>
> One of the considerations is just to distinguish Profiles from standard
> ISA extensions.
>
>
>
>> diff --git a/gas/doc/as.texi b/gas/doc/as.texi
>> index 0b4109e1149..3240427fabe 100644
>> --- a/gas/doc/as.texi
>> +++ b/gas/doc/as.texi
>> @@ -557,7 +557,7 @@ gcc(1), ld(1), and the Info entries for
>> @file{binutils} and @file{ld}.
>>
>>  @emph{Target RISC-V options:}
>>     [@b{-fpic}|@b{-fPIC}|@b{-fno-pic}]
>> -   [@b{-march}=@var{ISA}]
>> +   [@b{-march}=@var{ISA/Profiles/Profiles+ISA}]
>
>     [@b{-mabi}=@var{ABI}]
>>     [@b{-mlittle-endian}|@b{-mbig-endian}]
>>  @end ifset
>> diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
>> index cec7457fe81..6334b637275 100644
>> --- a/gas/doc/c-riscv.texi
>> +++ b/gas/doc/c-riscv.texi
>> @@ -41,9 +41,10 @@ Generate position-independent code
>>  @item -fno-pic
>>  Don't generate position-independent code (default)
>>
>> -@cindex @samp{-march=ISA} option, RISC-V
>> -@item -march=ISA
>> -Select the base isa, as specified by ISA.  For example -march=rv32ima.
>> +@cindex @samp{-march=ISA/Profiles/Profies+ISA} option, RISC-V
>> +@item -march=ISA/Profiles/Profiles+ISA
>> +Select the base isa, as specified by ISA or Profiles or Profies+ISA.
>> +For example -march=rv32ima -march=RVI20U64 -march=RVI20U64+d.
>>  If this option and the architecture attributes aren't set, then assembler
>>  will check the default configure setting --with-arch=ISA.
>>
>
> The ISA means architecture string, and must be start with rv[32|64][i|e|g]
> before, so -march=Profiles+ISA looks confused since -march=rvi20u64+rv32i
> should be correct according to the rule, but in fact it's an illegal usage
> for the current implementation.  Personally, I think the profile strings
> belonged to the base isa may be a good idea, which is rv32i, rv32e, rv32g,
> rv64i, rv64g, rvi20u64, rvi20u32, rva20u64 or rva22u64.
>
> Thanks
> Nelson
>
> Thanks for your suggestions. In fact, we have discussed the format of
> profiles in -march in the riscv toolchain calling convention repository,and
> the final format is not fixed yet.
>
> https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/36
>

Thanks, that's great if toolchain convention records the rules.  Routhly
review that the llvm landed patch seems same as the above thoughts,
including,
1. The lower profile names
2. Just using `_' to separate profiles and extra extensions

So it would be great if GNU gcc/binutils also have the same implementations
:-)

Nelson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240909/3800da61/attachment-0001.htm>