| Message ID | 20231115013317.88282-1-patrick@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Fix ICE in non-canonical march parsing | expand |
LGTM, and BTW...I am thinking we could relax the canonical order during parsing, did you have interesting and time working on that item? On Wed, Nov 15, 2023 at 9:35 AM Patrick O'Neill <patrick@rivosinc.com> wrote: > > Passing in a base extension in non-canonical order (i, e, g) causes GCC > to ICE: > xgcc: error: '-march=rv64ge': ISA string is not in canonical order. 'e' > xgcc: internal compiler error: in add, at common/config/riscv/riscv-common.cc:671 > ... > > This is fixed by skipping to the next extension when a non-canonical > order is detected. > > gcc/ChangeLog: > > * common/config/riscv/riscv-common.cc > (riscv_subset_list::parse_std_ext): Emit an error and skip to > the next extension when a non-canonical ordering is detected. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/arch-27.c: New test. > * gcc.target/riscv/arch-28.c: New test. > > Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> > --- > Tested using rv64gc glibc on QEMU. > --- > gcc/common/config/riscv/riscv-common.cc | 17 +++++++++++++---- > gcc/testsuite/gcc.target/riscv/arch-24.c | 7 +++++++ > gcc/testsuite/gcc.target/riscv/arch-25.c | 7 +++++++ > 3 files changed, 27 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-24.c > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-25.c > > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc > index 884d81c12aa..66aea71ea2c 100644 > --- a/gcc/common/config/riscv/riscv-common.cc > +++ b/gcc/common/config/riscv/riscv-common.cc > @@ -964,15 +964,24 @@ riscv_subset_list::parse_std_ext (const char *p) > std_ext = *p; > > /* Checking canonical order. */ > + const char *prior_std_exts = std_exts; > + > while (*std_exts && std_ext != *std_exts) > std_exts++; > > 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); > + { > + error_at (m_loc, > + "%<-march=%s%>: ISA string is not in canonical order. " > + "%<%c%>", > + m_arch, *p); > + /* Extension ordering is invalid. Ignore this extension and keep > + searching for other issues with remaining extensions. */ > + std_exts = prior_std_exts; > + p++; > + continue; > + } > > std_exts++; > > diff --git a/gcc/testsuite/gcc.target/riscv/arch-24.c b/gcc/testsuite/gcc.target/riscv/arch-24.c > new file mode 100644 > index 00000000000..70143b2156f > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/arch-24.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64ge -mabi=lp64d" } */ > +int foo() > +{ > +} > + > +/* { dg-error "ISA string is not in canonical order. 'e'" "" { target *-*-* } 0 } */ > diff --git a/gcc/testsuite/gcc.target/riscv/arch-25.c b/gcc/testsuite/gcc.target/riscv/arch-25.c > new file mode 100644 > index 00000000000..934399a7b3a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/arch-25.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64imaefcv -mabi=lp64d" } */ > +int foo() > +{ > +} > + > +/* { dg-error "ISA string is not in canonical order. 'e'" "" { target *-*-* } 0 } */ > -- > 2.34.1 >
Does relax mean no longer enforcing the canonical order of extensions? Patrick On 11/14/23 17:52, Kito Cheng wrote: > LGTM, and BTW...I am thinking we could relax the canonical order > during parsing, did you have interesting and time working on that > item? > > On Wed, Nov 15, 2023 at 9:35 AM Patrick O'Neill<patrick@rivosinc.com> wrote: >> Passing in a base extension in non-canonical order (i, e, g) causes GCC >> to ICE: >> xgcc: error: '-march=rv64ge': ISA string is not in canonical order. 'e' >> xgcc: internal compiler error: in add, at common/config/riscv/riscv-common.cc:671 >> ... >> >> This is fixed by skipping to the next extension when a non-canonical >> order is detected. >> >> gcc/ChangeLog: >> >> * common/config/riscv/riscv-common.cc >> (riscv_subset_list::parse_std_ext): Emit an error and skip to >> the next extension when a non-canonical ordering is detected.
On Thu, Nov 16, 2023 at 2:32 AM Patrick O'Neill <patrick@rivosinc.com> wrote: > > Does relax mean no longer enforcing the canonical order of extensions? Yes, we've discussed that a long time ago, but we just didn't have enough people to moving that forward: https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14 > Patrick > > On 11/14/23 17:52, Kito Cheng wrote: > > LGTM, and BTW...I am thinking we could relax the canonical order > during parsing, did you have interesting and time working on that > item? > > On Wed, Nov 15, 2023 at 9:35 AM Patrick O'Neill <patrick@rivosinc.com> wrote: > > Passing in a base extension in non-canonical order (i, e, g) causes GCC > to ICE: > xgcc: error: '-march=rv64ge': ISA string is not in canonical order. 'e' > xgcc: internal compiler error: in add, at common/config/riscv/riscv-common.cc:671 > ... > > This is fixed by skipping to the next extension when a non-canonical > order is detected. > > gcc/ChangeLog: > > * common/config/riscv/riscv-common.cc > (riscv_subset_list::parse_std_ext): Emit an error and skip to > the next extension when a non-canonical ordering is detected.
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc index 884d81c12aa..66aea71ea2c 100644 --- a/gcc/common/config/riscv/riscv-common.cc +++ b/gcc/common/config/riscv/riscv-common.cc @@ -964,15 +964,24 @@ riscv_subset_list::parse_std_ext (const char *p) std_ext = *p; /* Checking canonical order. */ + const char *prior_std_exts = std_exts; + while (*std_exts && std_ext != *std_exts) std_exts++; 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); + { + error_at (m_loc, + "%<-march=%s%>: ISA string is not in canonical order. " + "%<%c%>", + m_arch, *p); + /* Extension ordering is invalid. Ignore this extension and keep + searching for other issues with remaining extensions. */ + std_exts = prior_std_exts; + p++; + continue; + } std_exts++; diff --git a/gcc/testsuite/gcc.target/riscv/arch-24.c b/gcc/testsuite/gcc.target/riscv/arch-24.c new file mode 100644 index 00000000000..70143b2156f --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/arch-24.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64ge -mabi=lp64d" } */ +int foo() +{ +} + +/* { dg-error "ISA string is not in canonical order. 'e'" "" { target *-*-* } 0 } */ diff --git a/gcc/testsuite/gcc.target/riscv/arch-25.c b/gcc/testsuite/gcc.target/riscv/arch-25.c new file mode 100644 index 00000000000..934399a7b3a --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/arch-25.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64imaefcv -mabi=lp64d" } */ +int foo() +{ +} + +/* { dg-error "ISA string is not in canonical order. 'e'" "" { target *-*-* } 0 } */
Passing in a base extension in non-canonical order (i, e, g) causes GCC to ICE: xgcc: error: '-march=rv64ge': ISA string is not in canonical order. 'e' xgcc: internal compiler error: in add, at common/config/riscv/riscv-common.cc:671 ... This is fixed by skipping to the next extension when a non-canonical order is detected. gcc/ChangeLog: * common/config/riscv/riscv-common.cc (riscv_subset_list::parse_std_ext): Emit an error and skip to the next extension when a non-canonical ordering is detected. gcc/testsuite/ChangeLog: * gcc.target/riscv/arch-27.c: New test. * gcc.target/riscv/arch-28.c: New test. Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> --- Tested using rv64gc glibc on QEMU. --- gcc/common/config/riscv/riscv-common.cc | 17 +++++++++++++---- gcc/testsuite/gcc.target/riscv/arch-24.c | 7 +++++++ gcc/testsuite/gcc.target/riscv/arch-25.c | 7 +++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.target/riscv/arch-24.c create mode 100644 gcc/testsuite/gcc.target/riscv/arch-25.c