[PATCH v2] RISC-V: Fixed the broken .option norvc and .option arch, -c

[Jan Beulich]

On 04.07.2025 11:02, Nelson Chu wrote:
> On Fri, Jul 4, 2025 at 3:33 PM Jan Beulich <jbeulich@suse.com> wrote:
> 
>> On 04.07.2025 09:19, Nelson Chu wrote:
>>> --- a/bfd/elfxx-riscv.c
>>> +++ b/bfd/elfxx-riscv.c
>>> @@ -2707,7 +2707,15 @@ riscv_update_subset1 (riscv_parse_subset_t *rps,
>>>       }
>>>
>>>        if (removed)
>>> -     riscv_remove_subset (rps->subset_list, subset);
>>> +     {
>>> +       riscv_remove_subset (rps->subset_list, subset);
>>> +
>>> +       /* FIXME: This is a workaround that only make the c removal
>> works,
>>> +          but should find a way to correct others.  */
>>> +       if (strcmp (subset, "c") == 0)
>>> +         riscv_update_subset (rps,
>> "-zca,-zcd,-zcf,-zcb,-zce,-zcmp,-zcmt,"
>>> +                                   "-zcmop,-zclsd");
>>> +     }
>>
>> On the assumption that this is the complete (current) set of Zc*
>> extensions,
>> this looks good to me
> 
> 
> Thanks for the review ;)
> 
> 
>> fwiw. What I keep forgetting is why the handling of C
>> isn't done using the normal dependency chains between extensions.
> 
> 
> Well... It was probably just an implementation mistake when dividing c into
> zca + zcf and zcd...  In fact we can,
> 
> 1. For riscv_implicit_subsets, ...
> 
> +static bool
> +check_implicit_for_c_zcf (riscv_parse_subset_t *rps,
> +  riscv_subset_t *subset ATTRIBUTE_UNUSED)
> +{
> +  return (rps != NULL
> +  && rps->xlen != NULL
> +  && *rps->xlen == 32
> +  && riscv_subset_supports (rps, "f"));
> +}
> +
> +static bool
> +check_implicit_for_c_zcd (riscv_parse_subset_t *rps,
> +  riscv_subset_t *subset ATTRIBUTE_UNUSED)
> +{
> +  return (rps != NULL
> +  && riscv_subset_supports (rps, "d"));
> + }
> +
>  static struct riscv_implicit_subset riscv_implicit_subsets[] =
>  {
> ...
> +  /* c = zca + zcf (if f) + zcd (if d).  */
> +  {"c", "+zcf", check_implicit_for_c_zcf},
> +  {"c", "+zcd", check_implicit_for_c_zcd},
> +  {"c", "+zca", check_implicit_always},
> ...
> }
> 
> 2. For opcode table,
> Changed INSN_CLASS_C to INSN_CLASS_ZCA
> Changed INSN_CLASS_F_AND_C to INSN_CLASS_ZCF
> Changed INSN_CLASS_D_AND_C to INSN_CLASS_ZCD
> 
> So ".option arch, -zca" should work since the base c instructions (without
> f and d) are all controlled by zca rather than c, even though the c is
> still in the arch string.  And the ".option arch, -zcf" and ".option arch,
> -zcd" also work as expected.  But the ".option arch, -c" seems to be
> ".option arch, -zca,-zcf,-zcd", and .option norvc should be ".option arch,
> -zca,-zcd,-zcf,-zcb,-zce,-zcmp,-zcmt,-zcmt,-zcmop,-zclsd".  I am thinking
> that we probably should only let .option norvc works for now, before we
> clarify the whole behavior of ".option arch, -ext"?

Fine with me, if getting the rest sorted goes significantly beyond the
outline you supplied above. I was merely wondering why, if RISC-V already
has that nice dependency scheme, it can't be made use of here. But as per
your reply I now understand there's still code missing for that to work.

Jan