[PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[Lehua Ding]

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.

e.g.:

Run `riscv64-unknown-elf-gcc -march=rv64gcv_zvl128_s123 a.c` will throw these error:

riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 'zvl' starts with `z` but is not a standard sub-extension
riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 's123' start with `s` but not a standard supervisor extension

Best,
Lehua

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc (standard_extensions_p): New func.
	(riscv_subset_list::add): Add check.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-3.c: Update -march.
	* 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-9.c: Ditto.
	* gcc.target/riscv/pr102957.c: Ditto.
	* gcc.target/riscv/arch-22.cc: New test.

---
 gcc/common/config/riscv/riscv-common.cc       | 29 +++++++++++++++++++
 gcc/testsuite/gcc.target/riscv/arch-22.cc     |  8 +++++
 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-9.c  |  4 +--
 gcc/testsuite/gcc.target/riscv/pr102957.c     |  2 ++
 8 files changed, 45 insertions(+), 6 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..df3c256c80c 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -518,6 +518,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 +558,23 @@ riscv_subset_list::add (const char *subset, int major_version,
 
       return;
     }
+  else if (subset[0] == 'z' && !standard_extensions_p (subset))
+    {
+      error_at (m_loc,
+		"%<-march=%s%>: extension %qs starts with `z` but is not a "
+		"standard sub-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 not a standard "
+	"supervisor extension",
+	m_arch, subset);
+      return;
+    }
 
   riscv_subset_t *s = new riscv_subset_t ();
   riscv_subset_t *itr;
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..f9d8b57cb20
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/arch-22.cc
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl128_z123_s123 -mabi=lp64d" } */
+int foo()
+{
+}
+/* { dg-error "extension 'zvl128' start with `z` but not a standard sub-extension" "" { target *-*-* } 0 } */
+/* { dg-error "extension 'z123' start with `z` but not a standard sub-extension" "" { target *-*-* } 0 } */
+/* { dg-error "extension 's123' start with `s` but not a standard supervisor 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..dee0fc6656d 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_xbar -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..8bdaa9d17b2 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_xbar -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-9.c b/gcc/testsuite/gcc.target/riscv/attribute-9.c
index 7e3741a827c..ec5bab963ae 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_xbar -mabi=ilp32e" } */
 int foo()
 {
 }
-/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_sabc_xbar\"" } } */
+/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_svinval1p0_xbar\"" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/pr102957.c b/gcc/testsuite/gcc.target/riscv/pr102957.c
index 9747dde3038..e11236b8504 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 not a standard sub-extension" "" { target *-*-* } 0 } */
-- 
2.36.1

Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[juzhe.zhong]

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

LGTM



juzhe.zhong@rivai.ai
 
From: Lehua Ding
Date: 2023-07-12 11:27
To: gcc-patches
CC: juzhe.zhong; rdapp.gcc; kito.cheng; palmer; jeffreyalaw
Subject: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension
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.
 
e.g.:
 
Run `riscv64-unknown-elf-gcc -march=rv64gcv_zvl128_s123 a.c` will throw these error:
 
riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 'zvl' starts with `z` but is not a standard sub-extension
riscv64-unknown-elf-gcc: error: '-march=rv64gcv_zvl128_s123': extension 's123' start with `s` but not a standard supervisor extension
 
Best,
Lehua
 
gcc/ChangeLog:
 
* common/config/riscv/riscv-common.cc (standard_extensions_p): New func.
(riscv_subset_list::add): Add check.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/arch-3.c: Update -march.
* 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-9.c: Ditto.
* gcc.target/riscv/pr102957.c: Ditto.
* gcc.target/riscv/arch-22.cc: New test.
 
---
gcc/common/config/riscv/riscv-common.cc       | 29 +++++++++++++++++++
gcc/testsuite/gcc.target/riscv/arch-22.cc     |  8 +++++
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-9.c  |  4 +--
gcc/testsuite/gcc.target/riscv/pr102957.c     |  2 ++
8 files changed, 45 insertions(+), 6 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..df3c256c80c 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -518,6 +518,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 +558,23 @@ riscv_subset_list::add (const char *subset, int major_version,
       return;
     }
+  else if (subset[0] == 'z' && !standard_extensions_p (subset))
+    {
+      error_at (m_loc,
+ "%<-march=%s%>: extension %qs starts with `z` but is not a "
+ "standard sub-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 not a standard "
+ "supervisor extension",
+ m_arch, subset);
+      return;
+    }
   riscv_subset_t *s = new riscv_subset_t ();
   riscv_subset_t *itr;
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..f9d8b57cb20
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/arch-22.cc
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl128_z123_s123 -mabi=lp64d" } */
+int foo()
+{
+}
+/* { dg-error "extension 'zvl128' start with `z` but not a standard sub-extension" "" { target *-*-* } 0 } */
+/* { dg-error "extension 'z123' start with `z` but not a standard sub-extension" "" { target *-*-* } 0 } */
+/* { dg-error "extension 's123' start with `s` but not a standard supervisor 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..dee0fc6656d 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_xbar -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..8bdaa9d17b2 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_xbar -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-9.c b/gcc/testsuite/gcc.target/riscv/attribute-9.c
index 7e3741a827c..ec5bab963ae 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_xbar -mabi=ilp32e" } */
int foo()
{
}
-/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_sabc_xbar\"" } } */
+/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_svinval1p0_xbar\"" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/pr102957.c b/gcc/testsuite/gcc.target/riscv/pr102957.c
index 9747dde3038..e11236b8504 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 not a standard sub-extension" "" { target *-*-* } 0 } */
-- 
2.36.1
 

Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[Jeff Law]

On 7/11/23 21:30, juzhe.zhong@rivai.ai wrote:
> LGTM
OK for the trunk.
jeff

Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[Palmer Dabbelt]

On Wed, 12 Jul 2023 09:02:06 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 7/11/23 21:30, juzhe.zhong@rivai.ai wrote:
>> LGTM
> OK for the trunk.

I'd like to make sure Kito is OK with this.  IIUC the "pass through 
unknown extensions" behavior is deliberate.  It's not what I would have 
done, but I didn't do it ;)

> jeff

Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[Jeff Law]

On 7/12/23 10:32, Palmer Dabbelt wrote:
> On Wed, 12 Jul 2023 09:02:06 PDT (-0700), jeffreyalaw@gmail.com wrote:
>>
>>
>> On 7/11/23 21:30, juzhe.zhong@rivai.ai wrote:
>>> LGTM
>> OK for the trunk.
> 
> I'd like to make sure Kito is OK with this.  IIUC the "pass through 
> unknown extensions" behavior is deliberate.  It's not what I would have 
> done, but I didn't do it ;)
OK.   Not sure what the rationale behind that would be, but Kito may 
have had a good reason.  Let's wait for his input.

jeff

Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[Kito Cheng]

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

Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

[Lehua Ding]

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

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: &nbsp;"Kito&nbsp;Cheng"<kito.cheng@gmail.com&gt;;
Date: &nbsp;Thu, Jul 13, 2023 10:04 AM
To: &nbsp;"Jeff Law"<jeffreyalaw@gmail.com&gt;; 
Cc: &nbsp;"palmer"<palmer@rivosinc.com&gt;; "juzhe.zhong"<juzhe.zhong@rivai.ai&gt;; "lehua.ding"<lehua.ding@rivai.ai&gt;; "gcc-patches"<gcc-patches@gcc.gnu.org&gt;; "Robin Dapp"<rdapp.gcc@gmail.com&gt;; 
Subject: &nbsp;Re: [PATCH] RISC-V: Throw compilation error for unknown sub-extension or supervisor extension

&nbsp;

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 :

---
&gt; 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].

----
&gt; 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