Thanks for review. I uploaded version V2, which addresses Kito's comments, along with two changes. The first is to reduce repeated errors, which are currently reported at least twice. The second is to report as many mistakes as possible.
V2 URL: https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624377.html Best, Lehua ------------------ Original ------------------ From: "Kito Cheng"<kito.ch...@gmail.com>; Date: Thu, Jul 13, 2023 10:04 AM To: "Jeff Law"<jeffreya...@gmail.com>; Cc: "palmer"<pal...@rivosinc.com>; "juzhe.zhong"<juzhe.zh...@rivai.ai>; "lehua.ding"<lehua.d...@rivai.ai>; "gcc-patches"<gcc-patches@gcc.gnu.org>; "Robin Dapp"<rdapp....@gmail.com>; Subject: Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension That's intentional before, since some time binutils may have supported that but the compiler doesn't, so GCC just bypasses that to binutils to let binutils reject those unknown extensions. But I am considering rejecting those extensions or adding more checks on the GCC side recently too, because accepting unknown extensions might cause problems on the architecture extension test macro[1], it makes the value become unreliable if the extension version info isn't in GCC yet. So I am OK with this change but two minor comments : --- > riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 'zvl' starts with `z` but is not a standard sub-extension I would like to say it's `unsupported standard extension` rather than `not a standard sub-extension`. Because some extensions have just become ratified but GCC is unsupported yet, so `not a standard sub-extension` might confuse IMO. and why `extension` rather than `sub-extension`: IIRC `sub-extension` was used as an official term long ago, but it is called standard extension now[2]. ---- > riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 's123' start with `s` but not a standard supervisor extension `is` is missing. ---- Also I would like to reject unknown single letter extensions and `x` extensions too, for the same reason as the other two, except that make the architecture extension test macro less useful. [1] https://github.com/riscv-non-isa/riscv-c-api-doc/blob/master/riscv-c-api.md#architecture-extension-test-macro [2] https://github.com/riscv/riscv-isa-manual/blob/main/src/naming.adoc#additional-standard-extension-names