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

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

[-- Attachment #1: Type: text/plain, Size: 4266 bytes --]

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.





>
> > 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;
> > +}
>