On Thu, 17 Nov 2022 18:14:18 PST (-0800), christoph.muell...@vrull.eu wrote:
On Wed, Nov 9, 2022 at 4:01 AM Palmer Dabbelt <pal...@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
That's a different problem: with Zihpm and Zicntr we're ignoring known extensions, so we can pretend the ISA didn't make a backwards incompatible change. That requires explicitly ignoring words in the ISA manual, which is something we've tried very hard to do in the past -- maybe less so these days, but IMO it's still worth calling out (see the __builtin_riscv_pause() doc patch, for example).
BR ChristophWe'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
