[PATCH 0/5] RISC-V: Relax the -march string for accept any order

[PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Kito Cheng]

Do you know how to build a ISA string with following extension?
- g
- c
- zba
- zbs
- svnapot
- zve64d
- zvl128b

Don't trial and error with your gcc and don't read RISC-V ISA spec! OK, I believe it's impossible for most people, even I work for RISC-V so many years, I remember most of the rule of the the canonical order, it's still hard to order that right in short time...

So I think it's time to relax that for the -march string inputs, since we have so many extension today, but we still keep the canonicalization within the compiler, because we need that to handle multi-lib and also it's easier to compare different ISA string.

This patch break into serveral part:
1) Small refactor patch
2) Change the way of parsing ISA string.
3) Remove unused functions
4) Update test cases
5) Update document

[PATCH 1/5] RISC-V: Extract part parsing base ISA logic into a standalone function [NFC]

[Kito Cheng]

Minor refactor, preparation for further change.

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	(riscv_subset_list::parse_base_ext): New.
	(riscv_subset_list::parse): Extract part of logic into
	riscv_subset_list::parse_base_ext.
	* config/riscv/riscv-subset.h (riscv_subset_list::parse_base_ext):
	New.
---
 gcc/common/config/riscv/riscv-common.cc | 68 ++++++++++++++++---------
 gcc/config/riscv/riscv-subset.h         |  2 +
 2 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 0301d170a41..f0359380451 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -970,25 +970,38 @@ riscv_subset_list::parsing_subset_version (const char *ext,
   return p;
 }
 
-/* Parsing function for standard extensions.
+/* Parsing function for base extensions, rv[32|64][i|e|g]
 
    Return Value:
-     Points to the end of extensions.
+     Points to the end of extensions, return NULL if any error.
 
    Arguments:
      `p`: Current parsing position.  */
-
 const char *
-riscv_subset_list::parse_std_ext (const char *p)
+riscv_subset_list::parse_base_ext (const char *p)
 {
-  const char *all_std_exts = riscv_supported_std_ext ();
-  const char *std_exts = all_std_exts;
-
   unsigned major_version = 0;
   unsigned minor_version = 0;
   char std_ext = '\0';
   bool explicit_version_p = false;
 
+  if (startswith (p, "rv32"))
+    {
+      m_xlen = 32;
+      p += 4;
+    }
+  else if (startswith (p, "rv64"))
+    {
+      m_xlen = 64;
+      p += 4;
+    }
+  else
+    {
+      error_at (m_loc, "%<-march=%s%>: ISA string must begin with rv32 or rv64",
+		m_arch);
+      return NULL;
+    }
+
   /* First letter must start with i, e or g.  */
   switch (*p)
     {
@@ -1043,6 +1056,28 @@ riscv_subset_list::parse_std_ext (const char *p)
 		"%<i%> or %<g%>", m_arch);
       return NULL;
     }
+  return p;
+}
+
+
+/* Parsing function for standard extensions.
+
+   Return Value:
+     Points to the end of extensions.
+
+   Arguments:
+     `p`: Current parsing position.  */
+
+const char *
+riscv_subset_list::parse_std_ext (const char *p)
+{
+  const char *all_std_exts = riscv_supported_std_ext ();
+  const char *std_exts = all_std_exts;
+
+  unsigned major_version = 0;
+  unsigned minor_version = 0;
+  char std_ext = '\0';
+  bool explicit_version_p = false;
 
   while (p != NULL && *p)
     {
@@ -1499,22 +1534,9 @@ riscv_subset_list::parse (const char *arch, location_t loc)
   riscv_subset_list *subset_list = new riscv_subset_list (arch, loc);
   riscv_subset_t *itr;
   const char *p = arch;
-  if (startswith (p, "rv32"))
-    {
-      subset_list->m_xlen = 32;
-      p += 4;
-    }
-  else if (startswith (p, "rv64"))
-    {
-      subset_list->m_xlen = 64;
-      p += 4;
-    }
-  else
-    {
-      error_at (loc, "%<-march=%s%>: ISA string must begin with rv32 or rv64",
-		arch);
-      goto fail;
-    }
+  p = subset_list->parse_base_ext (p);
+  if (p == NULL)
+    goto fail;
 
   /* Parsing standard extension.  */
   p = subset_list->parse_std_ext (p);
diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
index 14461838db5..c8117d8daf2 100644
--- a/gcc/config/riscv/riscv-subset.h
+++ b/gcc/config/riscv/riscv-subset.h
@@ -67,6 +67,8 @@ private:
   const char *parsing_subset_version (const char *, const char *, unsigned *,
 				      unsigned *, bool, bool *);
 
+  const char *parse_base_ext (const char *);
+
   const char *parse_std_ext (const char *);
 
   const char *parse_single_std_ext (const char *);
-- 
2.34.1

[PATCH 2/5] RISC-V: Relax the -march string for accept any order

[Kito Cheng]

-march was require canonical order before, however it's not easy for
most user when we have so many extension, so this patch is relax the
constraint, -march accept the ISA string in any order, it only has few
requirement:

1. Must start with rv[32|64][e|i|g].
2. Multi-letter and single letter extension must be separated by
   at least one underscore(`_`).

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	(riscv_subset_list::parse_single_std_ext): New parameter.
	(riscv_subset_list::parse_single_multiletter_ext): Ditto.
	(riscv_subset_list::parse_single_ext): Ditto.
	(riscv_subset_list::parse): Relax the order for the input of ISA
	string.
	* config/riscv/riscv-subset.h
	(riscv_subset_list::parse_single_std_ext): New parameter.
	(riscv_subset_list::parse_single_multiletter_ext): Ditto.
	(riscv_subset_list::parse_single_ext): Ditto.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-33.c: New.
	* gcc.target/riscv/arch-34.c: New.
---
 gcc/common/config/riscv/riscv-common.cc  | 91 ++++++++++++++----------
 gcc/config/riscv/riscv-subset.h          |  6 +-
 gcc/testsuite/gcc.target/riscv/arch-33.c |  5 ++
 gcc/testsuite/gcc.target/riscv/arch-34.c |  5 ++
 4 files changed, 67 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/arch-33.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/arch-34.c

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index f0359380451..891ecfce464 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1132,10 +1132,12 @@ riscv_subset_list::parse_std_ext (const char *p)
      Points to the end of extensions.
 
    Arguments:
-     `p`: Current parsing position.  */
+     `p`: Current parsing position.
+     `exact_single_p`: True if input string is exactly an extension and end
+     with '\0'.  */
 
 const char *
-riscv_subset_list::parse_single_std_ext (const char *p)
+riscv_subset_list::parse_single_std_ext (const char *p, bool exact_single_p)
 {
   if (*p == 'x' || *p == 's' || *p == 'z')
     {
@@ -1146,6 +1148,11 @@ riscv_subset_list::parse_single_std_ext (const char *p)
       return nullptr;
     }
 
+  if (exact_single_p && strlen (p) > 1)
+    {
+      return nullptr;
+    }
+
   unsigned major_version = 0;
   unsigned minor_version = 0;
   bool explicit_version_p = false;
@@ -1296,13 +1303,16 @@ riscv_subset_list::check_conflict_ext ()
    Arguments:
      `p`: Current parsing position.
      `ext_type`: What kind of extensions, 's', 'z' or 'x'.
-     `ext_type_str`: Full name for kind of extension.  */
+     `ext_type_str`: Full name for kind of extension.
+     `exact_single_p`: True if input string is exactly an extension and end
+     with '\0'.   */
 
 
 const char *
 riscv_subset_list::parse_single_multiletter_ext (const char *p,
 						 const char *ext_type,
-						 const char *ext_type_str)
+						 const char *ext_type_str,
+						 bool exact_single_p)
 {
   unsigned major_version = 0;
   unsigned minor_version = 0;
@@ -1314,6 +1324,7 @@ riscv_subset_list::parse_single_multiletter_ext (const char *p,
   char *subset = xstrdup (p);
   const char *end_of_version;
   bool explicit_version_p = false;
+  char *q = subset;
   char *ext;
   char backup;
   size_t len = strlen (p);
@@ -1321,6 +1332,17 @@ riscv_subset_list::parse_single_multiletter_ext (const char *p,
   bool found_any_number = false;
   bool found_minor_version = false;
 
+  if (!exact_single_p)
+    {
+      /* Extension may not ended with '\0', may come with another extension
+	 which concat by '_' */
+      /* Parse until end of this extension including version number.  */
+      while (*++q != '\0' && *q != '_')
+	;
+
+      len = q - subset;
+    }
+
   end_of_version_pos = len;
   /* Find the begin of version string.  */
   for (i = len -1; i > 0; --i)
@@ -1505,21 +1527,26 @@ riscv_subset_list::parse_multiletter_ext (const char *p,
      Points to the end of extensions.
 
    Arguments:
-     `p`: Current parsing position.  */
+     `p`: Current parsing position.
+     `exact_single_p`: True if input string is exactly an extension and end
+     with '\0'.  */
 
 const char *
-riscv_subset_list::parse_single_ext (const char *p)
+riscv_subset_list::parse_single_ext (const char *p, bool exact_single_p)
 {
   switch (p[0])
     {
     case 'x':
-      return parse_single_multiletter_ext (p, "x", "non-standard extension");
+      return parse_single_multiletter_ext (p, "x", "non-standard extension",
+					   exact_single_p);
     case 'z':
-      return parse_single_multiletter_ext (p, "z", "sub-extension");
+      return parse_single_multiletter_ext (p, "z", "sub-extension",
+					   exact_single_p);
     case 's':
-      return parse_single_multiletter_ext (p, "s", "supervisor extension");
+      return parse_single_multiletter_ext (p, "s", "supervisor extension",
+					   exact_single_p);
     default:
-      return parse_single_std_ext (p);
+      return parse_single_std_ext (p, exact_single_p);
     }
 }
 
@@ -1538,37 +1565,27 @@ riscv_subset_list::parse (const char *arch, location_t loc)
   if (p == NULL)
     goto fail;
 
-  /* Parsing standard extension.  */
-  p = subset_list->parse_std_ext (p);
-
-  if (p == NULL)
-    goto fail;
-
-  /* Parsing sub-extensions.  */
-  p = subset_list->parse_multiletter_ext (p, "z", "sub-extension");
-
-  if (p == NULL)
-    goto fail;
-
-  /* Parsing supervisor extension.  */
-  p = subset_list->parse_multiletter_ext (p, "s", "supervisor extension");
-
-  if (p == NULL)
-    goto fail;
-
-  /* Parsing non-standard extension.  */
-  p = subset_list->parse_multiletter_ext (p, "x", "non-standard extension");
+  while (p && *p)
+    {
+      switch (*p)
+	{
+	case '_':
+	  ++p;
+	  continue;
+	case 'e':
+	case 'i':
+	case 'g':
+	  error_at (loc, "%<-march=%s%>: 'i', 'e' or 'g' must be the first extension.",
+		    arch);
+	  goto fail;
+	default:
+	  p = subset_list->parse_single_ext (p, /*exact_single_p=*/ false);
+	}
+    }
 
   if (p == NULL)
     goto fail;
 
-  if (*p != '\0')
-    {
-      error_at (loc, "%<-march=%s%>: unexpected ISA string at end: %qs",
-               arch, p);
-      goto fail;
-    }
-
   for (itr = subset_list->m_head; itr != NULL; itr = itr->next)
     {
       subset_list->handle_implied_ext (itr->name.c_str ());
diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
index c8117d8daf2..a80f9138e5c 100644
--- a/gcc/config/riscv/riscv-subset.h
+++ b/gcc/config/riscv/riscv-subset.h
@@ -71,12 +71,12 @@ private:
 
   const char *parse_std_ext (const char *);
 
-  const char *parse_single_std_ext (const char *);
+  const char *parse_single_std_ext (const char *, bool);
 
   const char *parse_multiletter_ext (const char *, const char *,
 				     const char *);
   const char *parse_single_multiletter_ext (const char *, const char *,
-					    const char *);
+					    const char *, bool);
 
   void handle_implied_ext (const char *);
   bool check_implied_ext ();
@@ -101,7 +101,7 @@ public:
   riscv_subset_list *clone () const;
 
   static riscv_subset_list *parse (const char *, location_t);
-  const char *parse_single_ext (const char *);
+  const char *parse_single_ext (const char *, bool exact_single_p = true);
 
   const riscv_subset_t *begin () const {return m_head;};
   const riscv_subset_t *end () const {return NULL;};
diff --git a/gcc/testsuite/gcc.target/riscv/arch-33.c b/gcc/testsuite/gcc.target/riscv/arch-33.c
new file mode 100644
index 00000000000..3fac9285ca3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/arch-33.c
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64i_zba____zbs_mac_d -mabi=lp64d" } */
+int foo()
+{
+}
diff --git a/gcc/testsuite/gcc.target/riscv/arch-34.c b/gcc/testsuite/gcc.target/riscv/arch-34.c
new file mode 100644
index 00000000000..9b9f1a7a439
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/arch-34.c
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64i_xtheadbs_zba_fcd -mabi=lp64d" } */
+int foo()
+{
+}
-- 
2.34.1

[PATCH 3/5] RISC-V: Remove unused function in riscv_subset_list [NFC]

[Kito Cheng]

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	(riscv_subset_list::parse_std_ext): Remove.
	(riscv_subset_list::parse_multiletter_ext): Remove.
	* config/riscv/riscv-subset.h
	(riscv_subset_list::parse_std_ext): Remove.
	(riscv_subset_list::parse_multiletter_ext): Remove.
---
 gcc/common/config/riscv/riscv-common.cc | 179 ------------------------
 gcc/config/riscv/riscv-subset.h         |   4 -
 2 files changed, 183 deletions(-)

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 891ecfce464..cf1c82c9f5e 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1059,73 +1059,6 @@ riscv_subset_list::parse_base_ext (const char *p)
   return p;
 }
 
-
-/* Parsing function for standard extensions.
-
-   Return Value:
-     Points to the end of extensions.
-
-   Arguments:
-     `p`: Current parsing position.  */
-
-const char *
-riscv_subset_list::parse_std_ext (const char *p)
-{
-  const char *all_std_exts = riscv_supported_std_ext ();
-  const char *std_exts = all_std_exts;
-
-  unsigned major_version = 0;
-  unsigned minor_version = 0;
-  char std_ext = '\0';
-  bool explicit_version_p = false;
-
-  while (p != NULL && *p)
-    {
-      char subset[2] = {0, 0};
-
-      if (*p == 'x' || *p == 's' || *p == 'z')
-	break;
-
-      if (*p == '_')
-	{
-	  p++;
-	  continue;
-	}
-
-      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);
-	  /* 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++;
-
-      p++;
-
-      p = parsing_subset_version (subset, p, &major_version, &minor_version,
-				  /* std_ext_p= */ true, &explicit_version_p);
-
-      add (subset, major_version, minor_version, explicit_version_p, false);
-    }
-  return p;
-}
-
 /* Parsing function for one standard extensions.
 
    Return Value:
@@ -1409,118 +1342,6 @@ riscv_subset_list::parse_single_multiletter_ext (const char *p,
 
 }
 
-/* Parsing function for multi-letter extensions.
-
-   Return Value:
-     Points to the end of extensions.
-
-   Arguments:
-     `p`: Current parsing position.
-     `ext_type`: What kind of extensions, 's', 'z' or 'x'.
-     `ext_type_str`: Full name for kind of extension.  */
-
-const char *
-riscv_subset_list::parse_multiletter_ext (const char *p,
-					  const char *ext_type,
-					  const char *ext_type_str)
-{
-  unsigned major_version = 0;
-  unsigned minor_version = 0;
-  size_t ext_type_len = strlen (ext_type);
-
-  while (*p)
-    {
-      if (*p == '_')
-	{
-	  p++;
-	  continue;
-	}
-
-      if (strncmp (p, ext_type, ext_type_len) != 0)
-	break;
-
-      char *subset = xstrdup (p);
-      char *q = subset;
-      const char *end_of_version;
-      bool explicit_version_p = false;
-      char *ext;
-      char backup;
-      size_t len;
-      size_t end_of_version_pos, i;
-      bool found_any_number = false;
-      bool found_minor_version = false;
-
-      /* Parse until end of this extension including version number.  */
-      while (*++q != '\0' && *q != '_')
-	;
-
-      backup = *q;
-      *q = '\0';
-      len = q - subset;
-      *q = backup;
-
-      end_of_version_pos = len;
-      /* Find the begin of version string.  */
-      for (i = len -1; i > 0; --i)
-	{
-	  if (ISDIGIT (subset[i]))
-	    {
-	      found_any_number = true;
-	      continue;
-	    }
-	  /* Might be version seperator, but need to check one more char,
-	     we only allow <major>p<minor>, so we could stop parsing if found
-	     any more `p`.  */
-	  if (subset[i] == 'p' &&
-	      !found_minor_version &&
-	      found_any_number && ISDIGIT (subset[i-1]))
-	    {
-	      found_minor_version = true;
-	      continue;
-	    }
-
-	  end_of_version_pos = i + 1;
-	  break;
-	}
-
-      backup = subset[end_of_version_pos];
-      subset[end_of_version_pos] = '\0';
-      ext = xstrdup (subset);
-      subset[end_of_version_pos] = backup;
-
-      end_of_version
-	= parsing_subset_version (ext, subset + end_of_version_pos, &major_version, &minor_version,
-				  /* std_ext_p= */ false, &explicit_version_p);
-      free (ext);
-
-      if (end_of_version == NULL)
-	return NULL;
-
-      subset[end_of_version_pos] = '\0';
-
-      if (strlen (subset) == 1)
-	{
-	  error_at (m_loc, "%<-march=%s%>: name of %s must be more than 1 letter",
-		    m_arch, ext_type_str);
-	  free (subset);
-	  return NULL;
-	}
-
-      add (subset, major_version, minor_version, explicit_version_p, false);
-      p += end_of_version - subset;
-      free (subset);
-
-      if (*p != '\0' && *p != '_')
-	{
-	  error_at (m_loc, "%<-march=%s%>: %s must separate with %<_%>",
-		    m_arch, ext_type_str);
-	  return NULL;
-	}
-    }
-
-  return p;
-}
-
 /* Parsing function for a single-letter or multi-letter extensions.
 
    Return Value:
diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
index a80f9138e5c..ae849e2a302 100644
--- a/gcc/config/riscv/riscv-subset.h
+++ b/gcc/config/riscv/riscv-subset.h
@@ -69,12 +69,8 @@ private:
 
   const char *parse_base_ext (const char *);
 
-  const char *parse_std_ext (const char *);
-
   const char *parse_single_std_ext (const char *, bool);
 
-  const char *parse_multiletter_ext (const char *, const char *,
-				     const char *);
   const char *parse_single_multiletter_ext (const char *, const char *,
 					    const char *, bool);
 
-- 
2.34.1

[PATCH 4/5] RISC-V: Update testsuite due to -march string relaxation

[Kito Cheng]

We has relaxed -march string, it no longer require canonical order, so
we need update some of those testcase.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-23.c: Update test.
	* gcc.target/riscv/arch-27.c: Ditto.
	* gcc.target/riscv/arch-28.c: Ditto.
	* gcc.target/riscv/attribute-10.c: Ditto.
---
 gcc/testsuite/gcc.target/riscv/arch-23.c      | 1 -
 gcc/testsuite/gcc.target/riscv/arch-27.c      | 2 +-
 gcc/testsuite/gcc.target/riscv/arch-28.c      | 2 +-
 gcc/testsuite/gcc.target/riscv/attribute-10.c | 4 +++-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.target/riscv/arch-23.c b/gcc/testsuite/gcc.target/riscv/arch-23.c
index fca5425790c..aacfc451043 100644
--- a/gcc/testsuite/gcc.target/riscv/arch-23.c
+++ b/gcc/testsuite/gcc.target/riscv/arch-23.c
@@ -4,7 +4,6 @@ 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 'zvl' starts with 'z' but is unsupported standard extension" "" { target *-*-* } 0 } */
 /* { dg-error "extension 's123' starts with 's' but is unsupported standard supervisor extension" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/riscv/arch-27.c b/gcc/testsuite/gcc.target/riscv/arch-27.c
index 70143b2156f..03f07deedd1 100644
--- a/gcc/testsuite/gcc.target/riscv/arch-27.c
+++ b/gcc/testsuite/gcc.target/riscv/arch-27.c
@@ -4,4 +4,4 @@ int foo()
 {
 }
 
-/* { dg-error "ISA string is not in canonical order. 'e'" "" { target *-*-* } 0 } */
+/* { dg-error "'i', 'e' or 'g' must be the first extension" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/riscv/arch-28.c b/gcc/testsuite/gcc.target/riscv/arch-28.c
index 934399a7b3a..0f83c03ad3d 100644
--- a/gcc/testsuite/gcc.target/riscv/arch-28.c
+++ b/gcc/testsuite/gcc.target/riscv/arch-28.c
@@ -4,4 +4,4 @@ int foo()
 {
 }
 
-/* { dg-error "ISA string is not in canonical order. 'e'" "" { target *-*-* } 0 } */
+/* { dg-error "'i', 'e' or 'g' must be the first extension" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-10.c b/gcc/testsuite/gcc.target/riscv/attribute-10.c
index 868adef6ab7..8a7f0a8ac49 100644
--- a/gcc/testsuite/gcc.target/riscv/attribute-10.c
+++ b/gcc/testsuite/gcc.target/riscv/attribute-10.c
@@ -3,4 +3,6 @@
 int foo()
 {
 }
-/* { dg-error "unexpected ISA string at end:" "" { target { "riscv*-*-*" } } 0 } */
+/* { dg-error "extension 'u' is unsupported standard single letter extension" "" { target { "riscv*-*-*" } } 0 } */
+/* { dg-error "extension 'n' is unsupported standard single letter extension" "" { target { "riscv*-*-*" } } 0 } */
+/* { dg-error "'i', 'e' or 'g' must be the first extension" "" { target { "riscv*-*-*" } } 0 } */
-- 
2.34.1

[PATCH 5/5] RISC-V: Document the syntax of -march

[Kito Cheng]

---
 gcc/doc/invoke.texi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 68d1f364ac0..81ee7ac758a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -30037,6 +30037,22 @@ Generate code for given RISC-V ISA (e.g.@: @samp{rv64im}).  ISA strings must be
 lower-case.  Examples include @samp{rv64i}, @samp{rv32g}, @samp{rv32e}, and
 @samp{rv32imaf}.
 
+The syntax of the ISA string is defined as follows:
+
+@table @code
+@item The string must start with @samp{rv32} or @samp{rv64}, followed by
+@samp{i}, @samp{e}, or @samp{g}, referred to as the base ISA.
+@item The subsequent part of the string is a list of extension names. Extension
+names can be categorized as multi-letter (e.g.@: @samp{zba}) and single-letter
+(e.g.@: @samp{v}). Single-letter extensions can appear consecutively,
+but multi-letter extensions must be separated by underscores.
+@item An underscore can appear anywhere after the base ISA. It has no specific
+effect but is used to improve readability and can act as a separator.
+@item Extension names may include an optional version number, following the
+syntax @samp{<major>p<minor>} or @samp{<major>}, (e.g.@: @samp{m2p1} or
+@samp{m2}).
+@end table
+
 When @option{-march=} is not specified, use the setting from @option{-mcpu}.
 
 If both @option{-march} and @option{-mcpu=} are not specified, the default for
-- 
2.34.1

Re: [PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Jeff Law]

On 1/8/24 06:47, Kito Cheng wrote:
> 
> Do you know how to build a ISA string with following extension?
> - g
> - c
> - zba
> - zbs
> - svnapot
> - zve64d
> - zvl128b
> 
> Don't trial and error with your gcc and don't read RISC-V ISA spec! OK, I believe it's impossible for most people, even I work for RISC-V so many years, I remember most of the rule of the the canonical order, it's still hard to order that right in short time...
> 
> So I think it's time to relax that for the -march string inputs, since we have so many extension today, but we still keep the canonicalization within the compiler, because we need that to handle multi-lib and also it's easier to compare different ISA string.
> 
> This patch break into serveral part:
> 1) Small refactor patch
> 2) Change the way of parsing ISA string.
> 3) Remove unused functions
> 4) Update test cases
> 5) Update document
Just because something is hard doesn't necessarily mean we should avoid it.

A great example would be strict aliasing.  I'd bet that 90% of C/C++ 
developers would get something wrong in this space.  Similarly for 
oddities of FP arithmetic.

My biggest worry is consistency across various tools.  It's rather lame 
if GCC were on an island by itself either in being too strict or too loose.

So where are the other key tools in this regard?  Are we an outlier 
right now or will this patch make us an outlier?

jeff

Re: [PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Kito Cheng]

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

Oops, I should leave more context here:

Actually we discussed that years ago, and most people agree with that, but
I guess we are just missing that, and also the ISA string isn't so
terribly long yet at that moment, however...the number of extensions are
growth so fast in last year, so I think it's time to moving this forward.

Also we (SiFive) will send patches for clang/LLVM to relax that as well :)

https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14

On Wed, Jan 10, 2024 at 2:31 AM Jeff Law <jeffreyalaw@gmail.com> wrote:

>
>
> On 1/8/24 06:47, Kito Cheng wrote:
> >
> > Do you know how to build a ISA string with following extension?
> > - g
> > - c
> > - zba
> > - zbs
> > - svnapot
> > - zve64d
> > - zvl128b
> >
> > Don't trial and error with your gcc and don't read RISC-V ISA spec! OK,
> I believe it's impossible for most people, even I work for RISC-V so many
> years, I remember most of the rule of the the canonical order, it's still
> hard to order that right in short time...
> >
> > So I think it's time to relax that for the -march string inputs, since
> we have so many extension today, but we still keep the canonicalization
> within the compiler, because we need that to handle multi-lib and also it's
> easier to compare different ISA string.
> >
> > This patch break into serveral part:
> > 1) Small refactor patch
> > 2) Change the way of parsing ISA string.
> > 3) Remove unused functions
> > 4) Update test cases
> > 5) Update document
> Just because something is hard doesn't necessarily mean we should avoid it.
>
> A great example would be strict aliasing.  I'd bet that 90% of C/C++
> developers would get something wrong in this space.  Similarly for
> oddities of FP arithmetic.
>
> My biggest worry is consistency across various tools.  It's rather lame
> if GCC were on an island by itself either in being too strict or too loose.
>
> So where are the other key tools in this regard?  Are we an outlier
> right now or will this patch make us an outlier?
>
> jeff
>

Re: [PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Fangrui Song]

On Tue, Jan 9, 2024 at 4:59 PM Kito Cheng <kito.cheng@sifive.com> wrote:
>
> Oops, I should leave more context here:
>
> Actually we discussed that years ago, and most people agree with that, but I guess we are just missing that, and also the ISA string isn't so terribly long yet at that moment, however...the number of extensions are growth so fast in last year, so I think it's time to moving this forward.
>
> Also we (SiFive) will send patches for clang/LLVM to relax that as well :)
>
> https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14
>
> On Wed, Jan 10, 2024 at 2:31 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>>
>>
>> On 1/8/24 06:47, Kito Cheng wrote:
>> >
>> > Do you know how to build a ISA string with following extension?
>> > - g
>> > - c
>> > - zba
>> > - zbs
>> > - svnapot
>> > - zve64d
>> > - zvl128b
>> >
>> > Don't trial and error with your gcc and don't read RISC-V ISA spec! OK, I believe it's impossible for most people, even I work for RISC-V so many years, I remember most of the rule of the the canonical order, it's still hard to order that right in short time...
>> >
>> > So I think it's time to relax that for the -march string inputs, since we have so many extension today, but we still keep the canonicalization within the compiler, because we need that to handle multi-lib and also it's easier to compare different ISA string.
>> >
>> > This patch break into serveral part:
>> > 1) Small refactor patch
>> > 2) Change the way of parsing ISA string.
>> > 3) Remove unused functions
>> > 4) Update test cases
>> > 5) Update document
>> Just because something is hard doesn't necessarily mean we should avoid it.
>>
>> A great example would be strict aliasing.  I'd bet that 90% of C/C++
>> developers would get something wrong in this space.  Similarly for
>> oddities of FP arithmetic.
>>
>> My biggest worry is consistency across various tools.  It's rather lame
>> if GCC were on an island by itself either in being too strict or too loose.
>>
>> So where are the other key tools in this regard?  Are we an outlier
>> right now or will this patch make us an outlier?
>>
>> jeff

If we had fewer extensions, ensuring a canonical order is better as a
code search of a fixed string will retrieve the relevant results, and
I'd wish that we did not lose the strictness.
Now that there are a hundred extensions, I agree that enforcing a
strict order has lost its goodness...


-- 
宋方睿

Re: [PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Jeff Law]

On 1/9/24 17:58, Kito Cheng wrote:
> Oops, I should leave more context here:
> 
> Actually we discussed that years ago, and most people agree with that, 
> but I guess we are just missing that, and also the ISA string isn't so 
> terribly long yet at that moment, however...the number of extensions are 
> growth so fast in last year, so I think it's time to moving this forward.
> 
> Also we (SiFive) will send patches for clang/LLVM to relax that as well :)
> 
> https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14 
> <https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14>
Then let's go forward.  It seems like as good a time as any with gcc-14 
and llvm-18 both right around the corner.

jeff

Re: [PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Kito Cheng]

Pushed to trunk :)

On Tue, Jan 16, 2024 at 10:33 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 1/9/24 17:58, Kito Cheng wrote:
> > Oops, I should leave more context here:
> >
> > Actually we discussed that years ago, and most people agree with that,
> > but I guess we are just missing that, and also the ISA string isn't so
> > terribly long yet at that moment, however...the number of extensions are
> > growth so fast in last year, so I think it's time to moving this forward.
> >
> > Also we (SiFive) will send patches for clang/LLVM to relax that as well :)
> >
> > https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14
> > <https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14>
> Then let's go forward.  It seems like as good a time as any with gcc-14
> and llvm-18 both right around the corner.
>
> jeff

Re: [PATCH 0/5] RISC-V: Relax the -march string for accept any order

[Kito Cheng]

Oh, ok, I must have missed something during testing.

On Fri, Jan 19, 2024 at 5:37 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> Hi, kito.
>
> I found these following regression:
>
> FAIL: gcc.target/riscv/arch-27.c   -O0   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -O0  (test for excess errors)
> FAIL: gcc.target/riscv/arch-27.c   -O1   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -O1  (test for excess errors)
> FAIL: gcc.target/riscv/arch-27.c   -O2   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -O2  (test for excess errors)
> FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
> FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
> FAIL: gcc.target/riscv/arch-27.c   -O3 -g   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -O3 -g  (test for excess errors)
> FAIL: gcc.target/riscv/arch-27.c   -Os   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-27.c   -Os  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -O0   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -O0  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -O1   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -O1  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -O2   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -O2  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -O3 -g   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -O3 -g  (test for excess errors)
> FAIL: gcc.target/riscv/arch-28.c   -Os   at line 7 (test for errors, line )
> FAIL: gcc.target/riscv/arch-28.c   -Os  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -O0   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -O0  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -O1   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -O1  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -O2   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -O2  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -O3 -g   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -O3 -g  (test for excess errors)
> FAIL: gcc.target/riscv/attribute-10.c   -Os   at line 8 (test for errors, line )
> FAIL: gcc.target/riscv/attribute-10.c   -Os  (test for excess errors)
>
> Could you take a look at it ?
> I am not sure whether they are caused by this patch.  But I find only this patch looks related.
> ________________________________
> juzhe.zhong@rivai.ai

[PATCH 0/5] RISC-V: Relax the -march string for accept any order

[juzhe.zhong]

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

Hi, kito.

I found these following regression:

FAIL: gcc.target/riscv/arch-27.c   -O0   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -O0  (test for excess errors)
FAIL: gcc.target/riscv/arch-27.c   -O1   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -O1  (test for excess errors)
FAIL: gcc.target/riscv/arch-27.c   -O2   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -O2  (test for excess errors)
FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
FAIL: gcc.target/riscv/arch-27.c   -O3 -g   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -O3 -g  (test for excess errors)
FAIL: gcc.target/riscv/arch-27.c   -Os   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-27.c   -Os  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -O0   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -O0  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -O1   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -O1  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -O2   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -O2  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -O3 -g   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -O3 -g  (test for excess errors)
FAIL: gcc.target/riscv/arch-28.c   -Os   at line 7 (test for errors, line )
FAIL: gcc.target/riscv/arch-28.c   -Os  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -O0   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -O0  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -O1   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -O1  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -O2   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -O2  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -O3 -g   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -O3 -g  (test for excess errors)
FAIL: gcc.target/riscv/attribute-10.c   -Os   at line 8 (test for errors, line )
FAIL: gcc.target/riscv/attribute-10.c   -Os  (test for excess errors)

Could you take a look at it ?
I am not sure whether they are caused by this patch.  But I find only this patch looks related.


juzhe.zhong@rivai.ai