Re: [PATCH] RISC-V: Add the Zihpm and Zicntr extensions

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

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

On Wed, Nov 9, 2022 at 4:01 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:

> These extensions were recently frozen [1].  As per Andrew's post [2]
> we're meant to ignore these in software, this just adds them to the list
> of allowed extensions and otherwise ignores them.  I added these under
> SPEC_CLASS_NONE even though the PDF lists them as 20190614 because it
> seems pointless to add another spec class just to accept two extensions
> we then ignore.
>
> 1:
> https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/HZGoqP1eyps/m/GTNKRLJoAQAJ
> 2:
> https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/QKjQhChrq9Q/m/7gqdkctgAgAJ
>
> gcc/ChangeLog
>
>         * common/config/riscv/riscv-common.cc: Add Zihpm and Zicnttr
>         extensions.
>
> ---
>
> These deserves documentation, a test case, and a NEWS entry.  I didn't
> write those yet because it's not super clear this is the way we wanted
> to go, though: just flat out ignoring the ISA feels like the wrong thing
> to do, but the guidance here is pretty clear.  Still feels odd, though.
>


We already have the infrastructure in GAS to check the CSR numbers.
It is an optional feature, but it is here and working.
We follow the guidance in the default configuration (CSR checking needs to
be turned on).
As long as we want to keep this infrastructure, there is no question if we
should continue
to support new extensions as required by this feature:
We have to because everything else will lead to a broken feature.

The question if CSR checking in GAS should be removed or not does not have
to be
answered right now if there is doubt about making the wrong decision.

Additionally, I fully agree that we can not ignore unknown extensions.
We must report an unknown extension in the march string to the user.
And even without CSR checking, GCC needs to be aware of all extensions
(e.g. for possible future support of -march=native).

So I think this patch should go in (together with a test).

That's why I also sent something similar for Smaia and Ssaia:
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606640.html

BR
Christoph





> We've also still got an open discussion on how we want to handle -march
> going forwards that's pretty relevant here, so I figured it'd be best to
> send this out sooner rather than later as it's sort of related.
> ---
>  gcc/common/config/riscv/riscv-common.cc | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc
> b/gcc/common/config/riscv/riscv-common.cc
> index 4b7f777c103..72981f05ac7 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -190,6 +190,9 @@ static const struct riscv_ext_version
> riscv_ext_version_table[] =
>    {"zicbom",ISA_SPEC_CLASS_NONE, 1, 0},
>    {"zicbop",ISA_SPEC_CLASS_NONE, 1, 0},
>
> +  {"zicntr", ISA_SPEC_CLASS_NONE, 2, 0},
> +  {"zihpm",  ISA_SPEC_CLASS_NONE, 2, 0},
> +
>    {"zk",    ISA_SPEC_CLASS_NONE, 1, 0},
>    {"zkn",   ISA_SPEC_CLASS_NONE, 1, 0},
>    {"zks",   ISA_SPEC_CLASS_NONE, 1, 0},
> --
> 2.38.1
>
>