Re: [PATCH v2] RISC-V: Handle multi-lib path correclty for linux

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

Committed to trunk

On Thu, May 4, 2023 at 4:03 PM Kito Cheng via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> RISC-V Linux encodes the ABI into the path, so in theory, we can only use that
> to select multi-lib paths, and no way to use different multi-lib paths between
> `rv32i/ilp32` and `rv32ima/ilp32`, we'll mapping both to `/lib/ilp32`.
>
> It's hard to do that with GCC's builtin multi-lib selection mechanism; builtin
> mechanism did the option string compare and then enumerate all possible reuse
> rules during the build time. However, it's impossible to RISC-V; we have a huge
> number of combinations of `-march`, so implementing a customized multi-lib
> selection becomes the only solution.
>
> Multi-lib configuration is only used for determines which ISA should be used
> when compiling the corresponding ABI variant after this patch.
>
> During the multi-lib selection stage, only consider -mabi as the only key to
> select the multi-lib path.
>
> gcc/ChangeLog:
>
>         * common/config/riscv/riscv-common.cc (riscv_select_multilib_by_abi): New.
>         (riscv_select_multilib): New.
>         (riscv_compute_multilib): Extract logic to riscv_select_multilib and
>         also handle select_by_abi.
>         * config/riscv/elf.h (RISCV_USE_CUSTOMISED_MULTI_LIB): Change it
>         to select_by_abi_arch_cmodel from 1.
>         * config/riscv/linux.h (RISCV_USE_CUSTOMISED_MULTI_LIB): Define.
>         * config/riscv/riscv-opts.h (enum riscv_multilib_select_kind): New.
>
> ---
> V2 Changes:
> - Fix some trivial issue cause I forgot to squash patches...
>
> This patch also plan backport to GCC 13 after landing to trunk.
>
> ---
>  gcc/common/config/riscv/riscv-common.cc | 128 ++++++++++++++++--------
>  gcc/config/riscv/elf.h                  |   2 +-
>  gcc/config/riscv/linux.h                |   2 +
>  gcc/config/riscv/riscv-opts.h           |   9 ++
>  4 files changed, 100 insertions(+), 41 deletions(-)
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index 309a52def75f..57a2a279ef53 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -1441,9 +1441,6 @@ riscv_multi_lib_check (int argc ATTRIBUTE_UNUSED,
>    return "";
>  }
>
> -/* We only override this in bare-metal toolchain.  */
> -#ifdef RISCV_USE_CUSTOMISED_MULTI_LIB
> -
>  /* Find last switch with the prefix, options are take last one in general,
>     return NULL if not found, and return the option value if found, it could
>     return empty string if the option has no value.  */
> @@ -1597,6 +1594,68 @@ riscv_check_conds (
>    return match_score + ok_count * 100;
>  }
>
> +static const char *
> +riscv_select_multilib_by_abi (
> +  const std::string &riscv_current_arch_str,
> +  const std::string &riscv_current_abi_str,
> +  const riscv_subset_list *subset_list, const struct switchstr *switches,
> +  int n_switches, const std::vector<riscv_multi_lib_info_t> &multilib_infos)
> +{
> +  for (size_t i = 0; i < multilib_infos.size (); ++i)
> +    if (riscv_current_abi_str == multilib_infos[i].abi_str)
> +      return xstrdup (multilib_infos[i].path.c_str ());
> +
> +  return NULL;
> +}
> +
> +static const char *
> +riscv_select_multilib (
> +  const std::string &riscv_current_arch_str,
> +  const std::string &riscv_current_abi_str,
> +  const riscv_subset_list *subset_list, const struct switchstr *switches,
> +  int n_switches, const std::vector<riscv_multi_lib_info_t> &multilib_infos)
> +{
> +  int match_score = 0;
> +  int max_match_score = 0;
> +  int best_match_multi_lib = -1;
> +  /* Try to decision which set we should used.  */
> +  /* We have 3 level decision tree here, ABI, check input arch/ABI must
> +     be superset of multi-lib arch, and other rest option checking.  */
> +  for (size_t i = 0; i < multilib_infos.size (); ++i)
> +    {
> +      /* Check ABI is same first.  */
> +      if (riscv_current_abi_str != multilib_infos[i].abi_str)
> +       continue;
> +
> +      /* Found a potential compatible multi-lib setting!
> +        Calculate the match score.  */
> +      match_score = subset_list->match_score (multilib_infos[i].subset_list);
> +
> +      /* Checking other cond in the multi-lib setting.  */
> +      match_score = riscv_check_conds (switches, n_switches, match_score,
> +                                      multilib_infos[i].conds);
> +
> +      /* Record highest match score multi-lib setting.  */
> +      if (match_score > max_match_score)
> +       {
> +         best_match_multi_lib = i;
> +         max_match_score = match_score;
> +       }
> +    }
> +
> +  if (best_match_multi_lib == -1)
> +    {
> +      riscv_no_matched_multi_lib = true;
> +      return NULL;
> +    }
> +  else
> +    return xstrdup (multilib_infos[best_match_multi_lib].path.c_str ());
> +}
> +
> +#ifndef RISCV_USE_CUSTOMISED_MULTI_LIB
> +#define RISCV_USE_CUSTOMISED_MULTI_LIB select_by_builtin
> +#endif
> +
>  /* Implement TARGET_COMPUTE_MULTILIB.  */
>  static const char *
>  riscv_compute_multilib (
> @@ -1609,6 +1668,11 @@ riscv_compute_multilib (
>    const char *multilib_exclusions ATTRIBUTE_UNUSED,
>    const char *multilib_reuse ATTRIBUTE_UNUSED)
>  {
> +  enum riscv_multilib_select_kind select_kind = RISCV_USE_CUSTOMISED_MULTI_LIB;
> +
> +  if (select_kind == select_by_builtin)
> +    return multilib_dir;
> +
>    const char *p;
>    const char *this_path;
>    size_t this_path_len;
> @@ -1672,7 +1736,13 @@ riscv_compute_multilib (
>         }
>
>        this_path_len = p - this_path;
> -      multilib_info.path = std::string (this_path, this_path_len);
> +      const char *multi_os_dir_pos
> +       = (const char *) memchr (this_path, ':', this_path_len);
> +      if (multi_os_dir_pos)
> +       multilib_info.path
> +         = std::string (this_path, multi_os_dir_pos - this_path);
> +      else
> +       multilib_info.path = std::string (this_path, this_path_len);
>
>        option_conds.clear ();
>        /* Pasrse option check list into vector<string>.
> @@ -1707,48 +1777,26 @@ riscv_compute_multilib (
>        p++;
>      }
>
> -  int match_score = 0;
> -  int max_match_score = 0;
> -  int best_match_multi_lib = -1;
> -  /* Try to decision which set we should used.  */
> -  /* We have 3 level decision tree here, ABI, check input arch/ABI must
> -     be superset of multi-lib arch, and other rest option checking.  */
> -  for (size_t i = 0; i < multilib_infos.size (); ++i)
> +  switch (select_kind)
>      {
> -      /* Check ABI is same first.  */
> -      if (riscv_current_abi_str != multilib_infos[i].abi_str)
> -       continue;
> -
> -      /* Found a potential compatible multi-lib setting!
> -        Calculate the match score.  */
> -      match_score = subset_list->match_score (multilib_infos[i].subset_list);
> -
> -      /* Checking other cond in the multi-lib setting.  */
> -      match_score = riscv_check_conds (switches,
> -                                      n_switches,
> -                                      match_score,
> -                                      multilib_infos[i].conds);
> -
> -      /* Record highest match score multi-lib setting.  */
> -      if (match_score > max_match_score)
> -       {
> -         best_match_multi_lib = i;
> -         max_match_score = match_score;
> -       }
> -    }
> -
> -  if (best_match_multi_lib == -1)
> -    {
> -      riscv_no_matched_multi_lib = true;
> -      return multilib_dir;
> +    case select_by_abi:
> +      return riscv_select_multilib (riscv_current_arch_str,
> +                                   riscv_current_abi_str, subset_list,
> +                                   switches, n_switches, multilib_infos);
> +    case select_by_abi_arch_cmodel:
> +      return riscv_select_multilib_by_abi (riscv_current_arch_str,
> +                                          riscv_current_abi_str, subset_list,
> +                                          switches, n_switches,
> +                                          multilib_infos);
> +    case select_by_builtin:
> +      gcc_unreachable ();
> +    default:
> +      gcc_unreachable ();
>      }
> -  else
> -    return xstrdup (multilib_infos[best_match_multi_lib].path.c_str ());
>  }
>
>  #undef TARGET_COMPUTE_MULTILIB
>  #define TARGET_COMPUTE_MULTILIB riscv_compute_multilib
> -#endif
>
>  vec<const char *>
>  riscv_get_valid_option_values (int option_code,
> diff --git a/gcc/config/riscv/elf.h b/gcc/config/riscv/elf.h
> index a725c00b6373..4b7e5c988ca2 100644
> --- a/gcc/config/riscv/elf.h
> +++ b/gcc/config/riscv/elf.h
> @@ -37,4 +37,4 @@ along with GCC; see the file COPYING3.  If not see
>  #undef  ENDFILE_SPEC
>  #define ENDFILE_SPEC "crtend%O%s"
>
> -#define RISCV_USE_CUSTOMISED_MULTI_LIB 1
> +#define RISCV_USE_CUSTOMISED_MULTI_LIB select_by_abi_arch_cmodel
> diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
> index 2fdfd930cf2d..3e625e0f8676 100644
> --- a/gcc/config/riscv/linux.h
> +++ b/gcc/config/riscv/linux.h
> @@ -66,3 +66,5 @@ along with GCC; see the file COPYING3.  If not see
>     "/usr/lib" XLEN_SPEC "/" ABI_SPEC "/ "      \
>     "/lib/ "                                    \
>     "/usr/lib/ "
> +
> +#define RISCV_USE_CUSTOMISED_MULTI_LIB select_by_abi
> diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> index 4207db240ea0..1b2e6de5e1b1 100644
> --- a/gcc/config/riscv/riscv-opts.h
> +++ b/gcc/config/riscv/riscv-opts.h
> @@ -82,6 +82,15 @@ enum riscv_autovec_lmul_enum {
>    RVV_M8 = 8
>  };
>
> +enum riscv_multilib_select_kind {
> +  /* Select multilib by builtin way.  */
> +  select_by_builtin,
> +  /* Select multilib by ABI, arch and code model.  */
> +  select_by_abi_arch_cmodel,
> +  /* Select multilib by ABI only.  */
> +  select_by_abi,
> +};
> +
>  #define MASK_ZICSR    (1 << 0)
>  #define MASK_ZIFENCEI (1 << 1)
>
> --
> 2.39.2
>