Re: [RFC] RISC-V: Add profile supports.

[Kito Cheng <kito.cheng@gmail.com>]

Could you add some test cases?

---

Parsing logic is kind of too adhoc, I would prefer using something
like the following code to prevent magic pointer arithmetic like p+6:

something like this:

Table of all profile names = {"RVA20U64", riscv_profile::RVA20U64, ...}

const char *rva20u64[] = {"m", "a", "f", "d",... NULL};

table of profile content =
{
  {riscv_profile::RVA20U64, rva20u64},
   ..
}

parse march ()
{
  if march is startswith
  else if ((profile = parse_proile(march)) != risv_profile::NOT_PROFILE)
     handle_profile (profile)
  else
     error
}

handle_profile (profile)
{
  use table of profile content to update ext.
}


On Wed, Nov 2, 2022 at 5:54 AM jiawei <jiawei@iscas.ac.cn> wrote:
>handle_profile
> Add two new function to handle profile input,
> "parse_profile" will check if a input into -march is
> legal, if it is then "handle_profile" will check the
> profile's type[I/M/A], year[20/22] and mode[U/S/M],
> set different extensions combine, just deal mandatory
> part currently.
>
> gcc/ChangeLog:
>
>         * common/config/riscv/riscv-common.cc
>         (riscv_subset_list::parse_profile): Check if profile name is valid or not.
>         (riscv_subset_list::parse_std_ext): If input of -march option is
>                                             a profile,skip first ISA check.
>         (riscv_subset_list::parse): Handle rofile input in -march.
>         (riscv_subset_list::handle_profile): Handle differen profiles
>                                              expand to extensions.
>         * config/riscv/riscv-subset.h: New function prototypes.
>
>
> ---
>  gcc/common/config/riscv/riscv-common.cc | 95 +++++++++++++++++++++++--
>  gcc/config/riscv/riscv-subset.h         |  5 +-
>  2 files changed, 94 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index 602491c638d..da06bd89144 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -777,6 +777,35 @@ riscv_subset_list::parsing_subset_version (const char *ext,
>    return p;
>  }
>
> +/* Parsing function for profile.
> +
> +   Return Value:
> +     Points to the end of profile.
> +
> +   Arguments:
> +     `p`: Current parsing position.  */
> +
> +const char *
> +riscv_subset_list::parse_profile (const char *p)
> +{
> +  if(*p == 'I' || *p == 'M' || *p == 'A'){
> +    p++;
> +    if(startswith (p, "20") || startswith (p, "22"))
> +      p += 2;
> +    if (*p == 'U' || *p == 'S' || *p == 'M')
> +      p++;
> +    if(startswith (p, "64") || startswith (p, "32")){
> +       p += 2;
> +       riscv_subset_list::handle_profile(p-6, p-4, p-3);
> +       return p;
> +    }
> +  }
> +  else
> +    error_at (m_loc, "%<-march=%s%>: Invalid profile.", m_arch);
> +  return NULL;
> +}
> +
> +
>  /* Parsing function for standard extensions.parse_std_ext
>

It's sort of too adhoc parsing the profile name, I would prefer using
something like the following code to prevent magic pointer arithmetic
like p+6.
something
Table of all profile names = {"RVA20U64", riscv_profile::RVA20U64, ...}

const char *rva20u64[] = {"m", "a", "f", "d",... NULL};

table of profile content =
{
  {riscv_profile::RVA20U64, rva20u64},
   ..
}

parse march ()
{
  if march is startswith
  else if ((profile = parse_proile(march)) != risv_profile::NOT_PROFILE)
     handle_profile (profile)
  else
     error
}

handle_profile (profile)
{
  ad
}

>     Return Value:
> @@ -786,7 +815,7 @@ riscv_subset_list::parsing_subset_version (const char *ext,
>       `p`: Current parsing position.  */
>
>  const char *
> -riscv_subset_list::parse_std_ext (const char *p)
> +riscv_subset_list::parse_std_ext (const char *p, bool isprofile)
>  {
>    const char *all_std_exts = riscv_supported_std_ext ();
>    const char *std_exts = all_std_exts;
> @@ -795,8 +824,8 @@ riscv_subset_list::parse_std_ext (const char *p)
>    unsigned minor_version = 0;
>    char std_ext = '\0';
>    bool explicit_version_p = false;
> -
> -  /* First letter must start with i, e or g.  */
> +  if (!isprofile){
> +    /* First letter must start with i, e or g.  */
>    switch (*p)
>      {
>      case 'i':
> @@ -850,6 +879,7 @@ riscv_subset_list::parse_std_ext (const char *p)
>                 "%<i%> or %<g%>", m_arch);
>        return NULL;
>      }
> +}
>
>    while (p != NULL && *p)
>      {
> @@ -1093,6 +1123,7 @@ 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;
> +  bool isprofile = false;
>    if (startswith (p, "rv32"))
>      {
>        subset_list->m_xlen = 32;
> @@ -1103,15 +1134,26 @@ riscv_subset_list::parse (const char *arch, location_t loc)
>        subset_list->m_xlen = 64;
>        p += 4;
>      }
> +  else if (startswith (p, "RV"))
> +    {
> +      if (startswith (p+6, "64"))
> +       subset_list->m_xlen = 64;
> +      else
> +       subset_list->m_xlen = 32;
> +      p += 2;
> +      /* Parsing profile name.  */
> +      p = subset_list->parse_profile (p);
> +      isprofile = true;handle_profile
> +    }
>    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;
>      }
>
>    /* Parsing standard extension.  */
> -  p = subset_list->parse_std_ext (p);
> +  p = subset_list->parse_std_ext (p,isprofile);
>
>    if (p == NULL)
>      goto fail;
> @@ -1349,6 +1391,49 @@ riscv_handle_option (struct gcc_options *opts,
>      }
>  }
>
> +/* Expand profile with defined mandatory extensions,
> +   M-type/mode is emtpy and set as base right now.  */
> +void riscv_subset_list::handle_profile(const char *profile_type,
> +                       const char *profile_year,
> +                       const char *profile_mode)
> +{
> +  add ("i", false);
> +  if(*profile_type == 'A'){
> +    add ("m", false);
> +    add ("a", false);
> +    add ("f", false);
> +    add ("d", false);
> +    add ("c", false);
> +    add ("ziccamoa", false);
> +    add ("ziccif", false);
> +    add ("zicclsm", false);
> +    add ("ziccrse", false);
> +    add ("zicntr", false);
> +    add ("zicsr", false);
> +
> +    if(*profile_mode == 'S')
> +      add ("zifencei", false);
> +
> +    if(*profile_year == '2')
> +    {
> +      add ("zihintpause", false);
> +      add ("zihpm", false);
> +      add ("zba", false);
> +      add ("zbb", false);
> +      add ("zbs", false);
> +      add ("zicbom", false);
> +      add ("zicbop", false);
> +      add ("zicboz", false);
> +      add ("zfhmin", false);
> +      add ("zkt", false);
> +      if(*profile_mode == 'S'){
> +       add ("svpbmt", false);
> +       add ("svinval", false);
> +      }
> +    }
> +  }
> +}
> +
>  /* Expand arch string with implied extensions.  */
>
>  const char *
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index 0bb3a9d29d0..303be5ed9ed 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -62,13 +62,16 @@ private:
>    const char *parsing_subset_version (const char *, const char *, unsigned *,
>                                       unsigned *, bool, bool *);
>
> -  const char *parse_std_ext (const char *);
> +  const char *parse_profile (const char *);
> +
> +  const char *parse_std_ext (const char *, bool);
>
>    const char *parse_multiletter_ext (const char *, const char *,
>                                      const char *);
>
>    void handle_implied_ext (riscv_subset_t *);
>    void handle_combine_ext ();
> +  void handle_profile(const char *, const char *, const char *);
>
>  public:
>    ~riscv_subset_list ();
> --
> 2.25.1
>