[PATCH] RISC-V: Fix ICE in non-canonical march parsing

[PATCH] RISC-V: Fix ICE in non-canonical march parsing

[Patrick O'Neill]

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

Re: [PATCH] RISC-V: Fix ICE in non-canonical march parsing

[Kito Cheng]

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
>

[Committed] RISC-V: Fix ICE in non-canonical march parsing

[Patrick O'Neill]

Updated testcase names and committed.

Thanks,
Patrick

---

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-27.c |  7 +++++++
 gcc/testsuite/gcc.target/riscv/arch-28.c |  7 +++++++
 3 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/arch-27.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/arch-28.c

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 526dbb7603b..57fe856063e 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1017,15 +1017,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-27.c b/gcc/testsuite/gcc.target/riscv/arch-27.c
new file mode 100644
index 00000000000..70143b2156f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/arch-27.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-28.c b/gcc/testsuite/gcc.target/riscv/arch-28.c
new file mode 100644
index 00000000000..934399a7b3a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/arch-28.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

Re: [PATCH] RISC-V: Fix ICE in non-canonical march parsing

[Patrick O'Neill]

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

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.

Re: [PATCH] RISC-V: Fix ICE in non-canonical march parsing

[Kito Cheng]

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.