Re: [PATCH] RISC-V: Add support for AIA ISA extensions (Ssaia and Smaia)

["Christoph Müllner" <christoph.muellner@vrull.eu>]

On Fri, Nov 18, 2022 at 10:08 AM Christoph Müllner
<christoph.muellner@vrull.eu> wrote:
>
>
>
> On Fri, Nov 18, 2022 at 6:09 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Thu, 17 Nov 2022 18:12:23 PST (-0800), christoph.muellner@vrull.eu wrote:
>> > From: Christoph Müllner <christoph.muellner@vrull.eu>
>> >
>> > This patch adds support for the two AIA ISA extensions Ssaia and Smaia.
>> > They are not relelvant for the compiler, but the assembler might want
>> > to validate the CSRs. Therefore, all this patch does is recognize the
>> > extension name, emit a feature macro (incl. a test).
>>
>> This is pretty far in the weeds, but the AIA PDF says
>>
>>     extension Smaia encompasses all added CSRs and all modifications to
>>     interrupt response behavior that the AIA specifies for a hart, over
>>     all privilege levels
>>
>> but only a subset of AIA has been frozen.  I think that's fine, assuming
>> we're decoupling ourselves from the ISA strings (and thus extension
>> names).  We just need to document it somewhere -- presumably invoke, but
>> that doesn't document anything else yet so we don't really have a
>> pattern to match.
>
>
> Thanks for highlighting this!
> We could model this such that Smaia implies Ssaia.
> Since the tool's interpretation of these extensions is "availability of extension's CSRs",
> this should work.
> But it is mostly irrelevant for GCC, as Binutils does the CSR checking, and we need
> to model it there.
>
> I see what you mean with the "subset of AIA has been frozen".
> I would expect that the draft chapters ("Duo-PLIC" and "IOMMU Support") will
> introduce new CSRs in the future. They might get included in separate extensions,
> be available only if another extension is enabled (like the hypervisor CSRs), or
> they will be put into the existing Smaia and Ssaia extensions.
> The last case is problematic, as it would change the behavior of the CSR checker.
> We could therefore document that the CSR checker strictly follows the latest
> specs and that changing behavior is possible for that reason.
> Not perfect, but reasonable and a method to permanently solve the recurring
> CSR discussions.

Palmer, since you did not respond since 9 days,
I tried to guess what you want to have documented and made a change in
invoke.texi:
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607326.html

The Binutils patch landed already:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ac8df5a1921904b3928429e696ad8b40c612f829

>
>
>
>
>>
>>
>> > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
>> > ---
>> >  gcc/common/config/riscv/riscv-common.cc |  2 ++
>> >  gcc/testsuite/gcc.target/riscv/smaia.c  | 13 +++++++++++++
>> >  gcc/testsuite/gcc.target/riscv/ssaia.c  | 13 +++++++++++++
>> >  3 files changed, 28 insertions(+)
>> >  create mode 100644 gcc/testsuite/gcc.target/riscv/smaia.c
>> >  create mode 100644 gcc/testsuite/gcc.target/riscv/ssaia.c
>> >
>> > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>> > index 4b7f777c103..674eded07b7 100644
>> > --- a/gcc/common/config/riscv/riscv-common.cc
>> > +++ b/gcc/common/config/riscv/riscv-common.cc
>> > @@ -219,6 +219,8 @@ static const struct riscv_ext_version riscv_ext_version_table[] =
>> >
>> >    {"zmmul", ISA_SPEC_CLASS_NONE, 1, 0},
>> >
>> > +  {"smaia", ISA_SPEC_CLASS_NONE, 1, 0},
>> > +  {"ssaia", ISA_SPEC_CLASS_NONE, 1, 0},
>> >    {"svinval", ISA_SPEC_CLASS_NONE, 1, 0},
>> >    {"svnapot", ISA_SPEC_CLASS_NONE, 1, 0},
>> >
>> > diff --git a/gcc/testsuite/gcc.target/riscv/smaia.c b/gcc/testsuite/gcc.target/riscv/smaia.c
>> > new file mode 100644
>> > index 00000000000..9ca80236245
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/riscv/smaia.c
>> > @@ -0,0 +1,13 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-options "-march=rv64gc_smaia" { target { rv64 } } } */
>> > +/* { dg-options "-march=rv32gc_smaia" { target { rv32 } } } */
>> > +
>> > +#ifndef __riscv_smaia
>> > +#error Feature macro not defined
>> > +#endif
>> > +
>> > +int
>> > +foo (int a)
>> > +{
>> > +  return a;
>> > +}
>> > diff --git a/gcc/testsuite/gcc.target/riscv/ssaia.c b/gcc/testsuite/gcc.target/riscv/ssaia.c
>> > new file mode 100644
>> > index 00000000000..b20e0eb10f5
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/riscv/ssaia.c
>> > @@ -0,0 +1,13 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-options "-march=rv64gc_ssaia" { target { rv64 } } } */
>> > +/* { dg-options "-march=rv32gc_ssaia" { target { rv32 } } } */
>> > +
>> > +#ifndef __riscv_ssaia
>> > +#error Feature macro not defined
>> > +#endif
>> > +
>> > +int
>> > +foo (int a)
>> > +{
>> > +  return a;
>> > +}