> From:Kito Cheng <kito.ch...@gmail.com>
> Send Time:2025 Jun. 9 (Mon.) 17:33
> To:yunzezhu<yunze...@linux.alibaba.com>
> CC:"gcc-patches"<gcc-patches@gcc.gnu.org>
> Subject:Re: [PATCH] [RFC] RISC-V: Add extra check to help choosing multilib 
> with equivalent arch.
> 
> 
> Oh, yeah, I got your point, I was just misreading, the march is
> rv32imac rather than rv32imafc, that is because of the complicated
> implication rule.
> 
> So I think maybe we should mark C-ext as a EXT_FLAG_MACRO
> 
> Then skip all EXT_FLAG_MACRO during riscv_subset_list::match_score?
> 
> something like that:
> 
> diff --git a/gcc/common/config/riscv/riscv-common.cc
> b/gcc/common/config/riscv/riscv-common.cc
> index 6b5440365e3..b536d3758ce 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -410,10 +410,14 @@ riscv_subset_list::match_score
> (riscv_subset_list *list) const
>           be complicated.
>      TODO: We might consider the version of each extension.  */
>   for (s = list->m_head; s != NULL; s = s->next)
> -    if (this->lookup (s->name.c_str ()) != NULL)
> -      score++;
> -    else
> -      return 0;
> +    {
> +      if (this-ext-is-macro-ext)
> +       continue;
> +      if (this->lookup (s->name.c_str ()) != NULL)
> +       score++;
> +      else
> +       return 0;
> +    }
> 
>   return score;
> }
> diff --git a/gcc/config/riscv/riscv-ext.def b/gcc/config/riscv/riscv-ext.def
> index 816acaa34f4..b7d443b0c27 100644
> --- a/gcc/config/riscv/riscv-ext.def
> +++ b/gcc/config/riscv/riscv-ext.def
> @@ -179,7 +179,7 @@ DEFINE_RISCV_EXT(
>   /* FLAG_GROUP */ base,
>   /* BITMASK_GROUP_ID */ 0,
>   /* BITMASK_BIT_POSITION*/ 2,
> -  /* EXTRA_EXTENSION_FLAGS */ 0)
> +  /* EXTRA_EXTENSION_FLAGS */ EXT_FLAG_MACRO)
> 
> DEFINE_RISCV_EXT(
>   /* NAME */ b,
> 
> 
> Could you give a try in this direction?

Hi Kito:

Apologies for replying late.

When trying the idea of skipping ext with EXT_FLAG_MACRO,
I found this method might produce some problems:
Since the function of implying dependent extensions from
ext with EXT_FLAG_MACRO works fine, target arch aka 'this'
in riscv_subset_list::match_score contains all implied extensions.
So in my opinion, marking c-ext in target arch as EXT_FLAG_MACRO
and skipping it in riscv_subset_list::match_score is not necessary,
because gcc could deal with situation that target arch contains
ext with EXT_FLAG_MACRO now while lib arch contains its dependent
extensions.

On the other side we can hardly imply extension with EXT_FLAG_MACRO
from single or union of dependent extensions because of more strict rules.
That is when we have one or more dependent extensions in target arch,
not always the ext with EXT_FLAG_MACRO can be implied, and
lib with arch contains that ext with EXT_FLAG_MACRO cannot be selected.
Actually this is the situation I hope to deal with in my patch.

I'm going to use target arch rv32imaf_zca as example again:
In target arch rv32imaf_zca c-ext cannot be implied because of exist of f-ext,
and therefore all libs contain c-ext like rv32imca cannot be select.
Here I have an alternative idea based on skipping ext with
EXT_FLAG_MACRO, that I tried to change to skip ext with EXT_FLAG_MACRO
in lib arch, but soon I found this might produce fault:
For example, if we skip c-ext in lib arch, both lib rv32imac and rv32imacf
could be select by target arch rv32imaf_zca, and rv32imacf would
be chosen due to higher score, but in fact rv32imacf contains zcf
that not exist in target arch.
So I think if we want to skip ext with EXT_FLAG_MACRO in lib arch,
dependent ext could be implied should be checked as well, and this
is almost what my rfc patch does. 

Another possible idea is to change current libs gcc uses
by replacing macro extensions with their implied dependent exts.
This solves problem like select lib for target rv32imaf_zca that lib
rv32ima_zca changed from rv32imac could be chosen,
but I'm not sure whether changing libs would produce unexpected failures.
It would be glad to hear from you. Thanks!

Best Regards,
Yunze Zhu

Reply via email to