LGTM, thanks :)
On Thu, Jul 13, 2023 at 4:33 PM Lehua Ding <lehua.d...@rivai.ai> wrote:
>
> Hi,
>
> This tiny patch add a check for extension starts with 'z' or 's' in `-march`
> option. Currently this unknown extension will be passed to the assembler,
> which
> then reports an error. With this patch, the compiler will throw a compilation
> error if the extension starts with 'z' or 's' is not a standard sub-extension
> or
> supervisor extension. Along with two extra 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.
>
> e.g.:
>
> Run `riscv64-unknown-elf-gcc -march=rv64gvcw_zvl128_s123_x123 -mabi=lp64d a.c`
> will throw these error:
>
> riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': ISA string is
> not in canonical order. 'c'
> riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 'w'
> is unsupported standard single letter extension
> riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension
> 'zvl128' start with `z` but is unsupported standard extension
> riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension
> 's123' start with `s` but is unsupported standard supervisor extension
> riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension
> 'x123' start with `x` but is unsupported non-standard extension
>
> Best,
> Lehua
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc (riscv_supported_std_ext): Init.
> (standard_extensions_p): Add check.
> (riscv_subset_list::add): Just return NULL if it failed before.
> (riscv_subset_list::parse_std_ext): Continue parse when find a error
> (riscv_subset_list::parse): Just return NULL if it failed before.
> * config/riscv/riscv-subset.h (class riscv_subset_list): Add field.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/arch-2.c: Update -march.
> * gcc.target/riscv/arch-3.c: Ditto.
> * gcc.target/riscv/arch-5.c: Ditto.
> * gcc.target/riscv/arch-8.c: Ditto.
> * gcc.target/riscv/attribute-10.c: Ditto.
> * gcc.target/riscv/attribute-18.c: Ditto.
> * gcc.target/riscv/attribute-19.c: Ditto.
> * gcc.target/riscv/attribute-8.c: Ditto.
> * gcc.target/riscv/attribute-9.c: Ditto.
> * gcc.target/riscv/pr102957.c: Ditto.
> * gcc.target/riscv/arch-22.cc: New test.
>
> ---
> gcc/common/config/riscv/riscv-common.cc | 68 +++++++++++++++----
> gcc/config/riscv/riscv-subset.h | 5 ++
> gcc/testsuite/gcc.target/riscv/arch-2.c | 2 +-
> gcc/testsuite/gcc.target/riscv/arch-22.cc | 11 +++
> gcc/testsuite/gcc.target/riscv/arch-3.c | 2 +-
> gcc/testsuite/gcc.target/riscv/arch-5.c | 2 +-
> gcc/testsuite/gcc.target/riscv/arch-8.c | 2 +-
> gcc/testsuite/gcc.target/riscv/attribute-10.c | 2 +-
> gcc/testsuite/gcc.target/riscv/attribute-18.c | 4 +-
> gcc/testsuite/gcc.target/riscv/attribute-19.c | 4 +-
> gcc/testsuite/gcc.target/riscv/attribute-8.c | 4 +-
> gcc/testsuite/gcc.target/riscv/attribute-9.c | 4 +-
> gcc/testsuite/gcc.target/riscv/pr102957.c | 2 +
> 13 files changed, 87 insertions(+), 25 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/arch-22.cc
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc
> b/gcc/common/config/riscv/riscv-common.cc
> index 6091d8f281b..9de7c54269e 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -311,6 +311,8 @@ static const char *riscv_tunes[] =
>
> static const char *riscv_supported_std_ext (void);
>
> +bool riscv_subset_list::parse_failed = false;
> +
> static riscv_subset_list *current_subset_list = NULL;
>
> const riscv_subset_list *riscv_current_subset_list ()
> @@ -518,6 +520,18 @@ subset_cmp (const std::string &a, const std::string &b)
> }
> }
>
> +/* Return true if EXT is a standard extension. */
> +
> +static bool
> +standard_extensions_p (const char *ext)
> +{
> + const riscv_ext_version *ext_ver;
> + for (ext_ver = &riscv_ext_version_table[0]; ext_ver->name != NULL;
> ++ext_ver)
> + if (strcmp (ext, ext_ver->name) == 0)
> + return true;
> + return false;
> +}
> +
> /* Add new subset to list. */
>
> void
> @@ -546,6 +560,38 @@ riscv_subset_list::add (const char *subset, int
> major_version,
>
> return;
> }
> + else if (strlen (subset) == 1 && !standard_extensions_p (subset))
> + {
> + error_at (m_loc,
> + "%<-march=%s%>: extension %qs is unsupported standard single "
> + "letter extension",
> + m_arch, subset);
> + return;
> + }
> + else if (subset[0] == 'z' && !standard_extensions_p (subset))
> + {
> + error_at (m_loc,
> + "%<-march=%s%>: extension %qs starts with `z` but is "
> + "unsupported standard extension",
> + m_arch, subset);
> + return;
> + }
> + else if (subset[0] == 's' && !standard_extensions_p (subset))
> + {
> + error_at (m_loc,
> + "%<-march=%s%>: extension %qs start with `s` but is "
> + "unsupported standard supervisor extension",
> + m_arch, subset);
> + return;
> + }
> + else if (subset[0] == 'x' && !standard_extensions_p (subset))
> + {
> + error_at (m_loc,
> + "%<-march=%s%>: extension %qs start with `x` but is "
> + "unsupported non-standard extension",
> + m_arch, subset);
> + return;
> + }
>
> riscv_subset_t *s = new riscv_subset_t ();
> riscv_subset_t *itr;
> @@ -919,22 +965,16 @@ riscv_subset_list::parse_std_ext (const char *p)
> while (*std_exts && std_ext != *std_exts)
> std_exts++;
>
> - if (std_ext != *std_exts)
> - {
> - if (strchr (all_std_exts, std_ext) == NULL)
> - error_at (m_loc, "%<-march=%s%>: unsupported ISA subset %<%c%>",
> - m_arch, *p);
> - else
> - error_at (m_loc,
> - "%<-march=%s%>: ISA string is not in canonical order. "
> - "%<%c%>", m_arch, *p);
> - return NULL;
> - }
> + subset[0] = std_ext;
> + if (std_ext != *std_exts && standard_extensions_p (subset))
> + error_at (m_loc,
> + "%<-march=%s%>: ISA string is not in canonical order. "
> + "%<%c%>",
> + m_arch, *p);
>
> std_exts++;
>
> p++;
> - subset[0] = std_ext;
>
> p = parsing_subset_version (subset, p, &major_version, &minor_version,
> /* std_ext_p= */ true, &explicit_version_p);
> @@ -1139,6 +1179,9 @@ riscv_subset_list::parse_multiletter_ext (const char *p,
> riscv_subset_list *
> riscv_subset_list::parse (const char *arch, location_t loc)
> {
> + if (riscv_subset_list::parse_failed)
> + return NULL;
> +
> riscv_subset_list *subset_list = new riscv_subset_list (arch, loc);
> riscv_subset_t *itr;
> const char *p = arch;
> @@ -1205,6 +1248,7 @@ riscv_subset_list::parse (const char *arch, location_t
> loc)
>
> fail:
> delete subset_list;
> + riscv_subset_list::parse_failed = true;
> return NULL;
> }
>
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index 92e4fb31692..6691997df7c 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -41,6 +41,11 @@ struct riscv_subset_t
> /* Subset list. */
> class riscv_subset_list
> {
> +public:
> + /* Because the parse method is called in several places, to prevent
> repeated
> + errors, use this flag to prevent it from repeating parse. */
> + static bool parse_failed;
> +
> private:
> /* Original arch string. */
> const char *m_arch;
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-2.c
> b/gcc/testsuite/gcc.target/riscv/arch-2.c
> index 8908b4b3a8c..e308bf7407c 100644
> --- a/gcc/testsuite/gcc.target/riscv/arch-2.c
> +++ b/gcc/testsuite/gcc.target/riscv/arch-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-march=rv32ixabc_xfoo -mabi=ilp32" } */
> +/* { dg-options "-march=rv32ixtheadba_xtheadsync -mabi=ilp32" } */
> int foo()
> {
> }
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-22.cc
> b/gcc/testsuite/gcc.target/riscv/arch-22.cc
> new file mode 100644
> index 00000000000..68337a70406
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/arch-22.cc
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gvcw_zvl128_s123_x123 -mabi=lp64d" } */
> +int foo()
> +{
> +}
> +
> +/* { dg-error "ISA string is not in canonical order. 'c'" "" { target *-*-*
> } 0 } */
> +/* { dg-error "extension 'w' is unsupported standard single letter
> extension" "" { target *-*-* } 0 } */
> +/* { dg-error "extension 'zvl128' start with `z` but is unsupported standard
> extension" "" { target *-*-* } 0 } */
> +/* { dg-error "extension 's123' start with `s` but is unsupported standard
> supervisor extension" "" { target *-*-* } 0 } */
> +/* { dg-error "extension 'x123' start with `x` but is unsupported
> non-standard extension" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-3.c
> b/gcc/testsuite/gcc.target/riscv/arch-3.c
> index 7aa945eca20..ac733746fd6 100644
> --- a/gcc/testsuite/gcc.target/riscv/arch-3.c
> +++ b/gcc/testsuite/gcc.target/riscv/arch-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-march=rv32isabc_xbar -mabi=ilp32" } */
> +/* { dg-options "-march=rv32isvinval_xtheadba -mabi=ilp32" } */
> int foo()
> {
> }
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-5.c
> b/gcc/testsuite/gcc.target/riscv/arch-5.c
> index 8258552214f..b8b6cd048f7 100644
> --- a/gcc/testsuite/gcc.target/riscv/arch-5.c
> +++ b/gcc/testsuite/gcc.target/riscv/arch-5.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-march=rv32i_zfoo_sabc_xbar -mabi=ilp32" } */
> +/* { dg-options "-march=rv32i_zmmul_svnapot_xtheadba -mabi=ilp32" } */
> int foo()
> {
> }
> diff --git a/gcc/testsuite/gcc.target/riscv/arch-8.c
> b/gcc/testsuite/gcc.target/riscv/arch-8.c
> index 1b9e51b0e12..ef557aeb673 100644
> --- a/gcc/testsuite/gcc.target/riscv/arch-8.c
> +++ b/gcc/testsuite/gcc.target/riscv/arch-8.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-march=rv32id_zicsr_zifence -mabi=ilp32" } */
> +/* { dg-options "-march=rv32id_zicsr_zifencei -mabi=ilp32" } */
> int foo()
> {
> }
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-10.c
> b/gcc/testsuite/gcc.target/riscv/attribute-10.c
> index 1e121a10753..868adef6ab7 100644
> --- a/gcc/testsuite/gcc.target/riscv/attribute-10.c
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-10.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-march=rv32i -march=rv32im_sx_unexpectedstring -mabi=ilp32"
> } */
> +/* { dg-options "-march=rv32i -march=rv32im_svnapot_unexpectedstring
> -mabi=ilp32" } */
> int foo()
> {
> }
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-18.c
> b/gcc/testsuite/gcc.target/riscv/attribute-18.c
> index 492360cf7c1..fddbf15fc3e 100644
> --- a/gcc/testsuite/gcc.target/riscv/attribute-18.c
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-18.c
> @@ -1,4 +1,4 @@
> /* { dg-do compile } */
> -/* { dg-options "-mriscv-attribute -march=rv64imafdcp -mabi=lp64d
> -misa-spec=2.2" } */
> +/* { dg-options "-mriscv-attribute -march=rv64imafdc -mabi=lp64d
> -misa-spec=2.2" } */
> int foo() {}
> -/* { dg-final { scan-assembler ".attribute arch,
> \"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p\"" } } */
> +/* { dg-final { scan-assembler ".attribute arch,
> \"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0\"" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-19.c
> b/gcc/testsuite/gcc.target/riscv/attribute-19.c
> index 562f8089361..8150452f5b1 100644
> --- a/gcc/testsuite/gcc.target/riscv/attribute-19.c
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-19.c
> @@ -1,4 +1,4 @@
> /* { dg-do compile } */
> -/* { dg-options "-mriscv-attribute -march=rv64imp0p9 -mabi=lp64
> -misa-spec=2.2" } */
> +/* { dg-options "-mriscv-attribute -march=rv64im -mabi=lp64 -misa-spec=2.2"
> } */
> int foo() {}
> -/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p0_m2p0_p0p9\"" } }
> */
> +/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p0_m2p0\"" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-8.c
> b/gcc/testsuite/gcc.target/riscv/attribute-8.c
> index 4c91b9e6369..2065f74b977 100644
> --- a/gcc/testsuite/gcc.target/riscv/attribute-8.c
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-8.c
> @@ -1,6 +1,6 @@
> /* { dg-do compile } */
> -/* { dg-options "-mriscv-attribute -march=rv32i2p0xabc_xv5 -mabi=ilp32" } */
> +/* { dg-options "-mriscv-attribute -march=rv32i2p0xtheadba_xtheadbb
> -mabi=ilp32" } */
> int foo()
> {
> }
> -/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_xabc_xv5p0\"" }
> } */
> +/* { dg-final { scan-assembler ".attribute arch,
> \"rv32i2p0_xtheadba1p0_xtheadbb1p0\"" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-9.c
> b/gcc/testsuite/gcc.target/riscv/attribute-9.c
> index 7e3741a827c..3c31848ed80 100644
> --- a/gcc/testsuite/gcc.target/riscv/attribute-9.c
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-9.c
> @@ -1,6 +1,6 @@
> /* { dg-do compile } */
> -/* { dg-options "-mriscv-attribute -march=rv32i2p0sabc_xbar -mabi=ilp32e" }
> */
> +/* { dg-options "-mriscv-attribute -march=rv32i2p0svinval_xtheadba
> -mabi=ilp32e" } */
> int foo()
> {
> }
> -/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_sabc_xbar\"" } }
> */
> +/* { dg-final { scan-assembler ".attribute arch,
> \"rv32i2p0_svinval1p0_xtheadba1p0\"" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/pr102957.c
> b/gcc/testsuite/gcc.target/riscv/pr102957.c
> index 9747dde3038..322d49c62d4 100644
> --- a/gcc/testsuite/gcc.target/riscv/pr102957.c
> +++ b/gcc/testsuite/gcc.target/riscv/pr102957.c
> @@ -3,3 +3,5 @@
> int foo()
> {
> }
> +
> +/* { dg-error "extension 'zb' starts with `z` but is unsupported standard
> extension" "" { target *-*-* } 0 } */
> --
> 2.36.1
>
