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

[Nelson Chu]

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"?

Thanks
Nelson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250704/e16b51e9/attachment.htm>