diff mbox series

[RFC,v2] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils

Message ID 20240613233059.1451117-1-patrick@rivosinc.com
State New
Headers show
Series [RFC,v2] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils | expand

Commit Message

Patrick O'Neill June 13, 2024, 11:30 p.m. UTC 
Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
check to upgrade Zaamo/Zalrsc to 'a' when the assember does not support it.

This change respects Zaamo/Zalrsc when generating code.

Testcases that check for the default isa string will fail with the old binutils
since zaamo/zalrsc aren't emitted anymore. All other Zaamo/Zalrsc testcases
pass.

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	(riscv_subset_list::to_string): Add toggle to promote Zaamo/Zalrsc
	extensions to 'a'.
	(riscv_arch_str): Ditto.
	(riscv_expand_arch): Ditto.
	(riscv_expand_arch_from_cpu): Ditto.
	(riscv_expand_arch_upgrade_exts): New function. Wrapper around
	riscv_expand_arch to preserve the function signature.
	(riscv_expand_arch_no_upgrade_exts): Ditto
	(riscv_expand_arch_from_cpu_upgrade_exts): New function. Wrapper around
	riscv_expand_arch_from_cpu to preserve the function signature.
	(riscv_expand_arch_from_cpu_no_upgrade_exts): Ditto.
	* config/riscv/riscv-protos.h (riscv_arch_str): Add toggle to function
	prototype.
	* config/riscv/riscv-subset.h: Ditto.
	* config/riscv/riscv-target-attr.cc (riscv_process_target_attr):
	* config/riscv/riscv.cc (riscv_emit_attribute):
	(riscv_declare_function_name):
	* config/riscv/riscv.h (riscv_expand_arch): Remove.
	(riscv_expand_arch_from_cpu): Ditto.
	(riscv_expand_arch_upgrade_exts): Add toggle wrapper functions.
	(riscv_expand_arch_no_upgrade_exts): Ditto.
	(riscv_expand_arch_from_cpu_upgrade_exts): Ditto.
	(riscv_expand_arch_from_cpu_no_upgrade_exts): Ditto.
	(EXTRA_SPEC_FUNCTIONS): Ditto.
	(OPTION_DEFAULT_SPECS): Use non-upgraded march string when invoking the
	compiler.
	(ASM_SPEC): Use upgraded march string when invoking the assembler.
	* config.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Add zaamo/zalrsc assembler check.

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
This change is way more invasive than the last one hence the RFC. I'd be happy
to iterate more on this or just commit v1 with the misc configure diffs removed.
---
 gcc/common/config/riscv/riscv-common.cc | 101 ++++++++++++++++++++++--
 gcc/config.in                           |   6 ++
 gcc/config/riscv/riscv-protos.h         |   3 +-
 gcc/config/riscv/riscv-subset.h         |   2 +-
 gcc/config/riscv/riscv-target-attr.cc   |   4 +-
 gcc/config/riscv/riscv.cc               |   7 +-
 gcc/config/riscv/riscv.h                |  46 ++++++-----
 gcc/configure                           |  31 ++++++++
 gcc/configure.ac                        |   5 ++
 9 files changed, 174 insertions(+), 31 deletions(-)

--
2.34.1

Comments

Andreas Schwab June 15, 2024, 7:58 a.m. UTC | #1 
../../gcc/common/config/riscv/riscv-common.cc: In member function 'std::string riscv_subset_list::to_string(bool, bool) const':
../../gcc/common/config/riscv/riscv-common.cc:997:37: error: 'a_subset' may be used uninitialized [-Werror=maybe-uninitialized]
  997 |           if (subset_cmp (a_subset->name, subset->name) > 0)
      |                           ~~~~~~~~~~^~~~
../../gcc/common/config/riscv/riscv-common.cc:923:19: note: 'a_subset' was declared here
  923 |   riscv_subset_t *a_subset;
      |                   ^~~~~~~~
cc1plus: all warnings being treated as errors
diff mbox series

Patch

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 78dfd6b1470..cdb390982c8 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -907,7 +907,7 @@  riscv_subset_list::add (const char *subset, bool implied_p)
    VERSION_P to determine append version info or not.  */

 std::string
-riscv_subset_list::to_string (bool version_p) const
+riscv_subset_list::to_string (bool version_p, bool upgrade_exts) const
 {
   std::ostringstream oss;
   oss << "rv" << m_xlen;
@@ -916,6 +916,11 @@  riscv_subset_list::to_string (bool version_p) const
   riscv_subset_t *subset;

   bool skip_zifencei = false;
+  bool upgrade_zaamo_zalrsc = false;
+  bool has_a_ext = false;
+  bool insert_a_ext = false;
+  bool inserted_a_ext = false;
+  riscv_subset_t *a_subset;
   bool skip_zicsr = false;
   bool i2p0 = false;

@@ -943,6 +948,31 @@  riscv_subset_list::to_string (bool version_p) const
      a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
   skip_zifencei = true;
 #endif
+#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
+  /* Upgrade Zaamo/Zalrsc extensions to 'a' since binutils 2.42 and earlier
+     don't recognize zaamo/zalrsc.  */
+  upgrade_zaamo_zalrsc = upgrade_exts;
+  if (upgrade_zaamo_zalrsc)
+    {
+      for (subset = m_head; subset != NULL; subset = subset->next)
+	{
+	  if (subset->name == "a")
+	    has_a_ext = true;
+	  if (subset->name == "zaamo" || subset->name == "zalrsc")
+	    insert_a_ext = true;
+	}
+      if (insert_a_ext && !has_a_ext)
+	{
+	  unsigned int major_version = 0, minor_version = 0;
+	  get_default_version ("a", &major_version, &minor_version);
+	  a_subset = new riscv_subset_t ();
+	  a_subset->name = "a";
+	  a_subset->implied_p = false;
+	  a_subset->major_version = major_version;
+	  a_subset->minor_version = minor_version;
+	}
+    }
+#endif

   for (subset = m_head; subset != NULL; subset = subset->next)
     {
@@ -954,6 +984,27 @@  riscv_subset_list::to_string (bool version_p) const
 	  subset->name == "zicsr")
 	continue;

+      if (upgrade_zaamo_zalrsc && subset->name == "zaamo")
+	continue;
+
+      if (upgrade_zaamo_zalrsc && subset->name == "zalrsc")
+	continue;
+
+      if (upgrade_zaamo_zalrsc && insert_a_ext && !has_a_ext && !inserted_a_ext)
+	{
+	  gcc_assert (a_subset);
+	  /* Insert `a` extension in cannonical order.  */
+	  if (subset_cmp (a_subset->name, subset->name) > 0)
+	    {
+	      oss << a_subset->name;
+	      if (version_p)
+		oss << a_subset->major_version
+		    << 'p'
+		    << a_subset->minor_version;
+	      inserted_a_ext = true;
+	    }
+	}
+
       /* For !version_p, we only separate extension with underline for
 	 multi-letter extension.  */
       if (!first &&
@@ -973,6 +1024,12 @@  riscv_subset_list::to_string (bool version_p) const
 	     << subset->minor_version;
     }

+  if (upgrade_zaamo_zalrsc && insert_a_ext && !has_a_ext)
+    {
+      gcc_assert (inserted_a_ext);
+      free (a_subset);
+    }
+
   return oss.str ();
 }

@@ -1587,10 +1644,10 @@  riscv_subset_list::finalize ()
 /* Return the current arch string.  */

 std::string
-riscv_arch_str (bool version_p)
+riscv_arch_str (bool version_p, bool upgrade_exts)
 {
   if (current_subset_list)
-    return current_subset_list->to_string (version_p);
+    return current_subset_list->to_string (version_p, upgrade_exts);
   else
     return std::string();
 }
@@ -1896,18 +1953,33 @@  riscv_handle_option (struct gcc_options *opts,

 const char *
 riscv_expand_arch (int argc ATTRIBUTE_UNUSED,
-		   const char **argv)
+		   const char **argv,
+		   bool upgrade_exts)
 {
   gcc_assert (argc == 1);
   location_t loc = UNKNOWN_LOCATION;
   riscv_parse_arch_string (argv[0], NULL, loc);
-  const std::string arch = riscv_arch_str (false);
+  const std::string arch = riscv_arch_str (/*version_p*/ false, upgrade_exts);
   if (arch.length())
     return xasprintf ("-march=%s", arch.c_str());
   else
     return "";
 }

+const char *
+riscv_expand_arch_upgrade_exts (int argc ATTRIBUTE_UNUSED,
+				const char **argv)
+{
+  return riscv_expand_arch (argc, argv, /*upgrade_exts*/ true);
+}
+
+const char *
+riscv_expand_arch_no_upgrade_exts (int argc ATTRIBUTE_UNUSED,
+				   const char **argv)
+{
+  return riscv_expand_arch (argc, argv, /*upgrade_exts*/ false);
+}
+
 /* Expand default -mtune option from -mcpu option, use default --with-tune value
    if -mcpu don't have valid value.  */

@@ -1927,7 +1999,8 @@  riscv_default_mtune (int argc, const char **argv)

 const char *
 riscv_expand_arch_from_cpu (int argc ATTRIBUTE_UNUSED,
-			    const char **argv)
+			    const char **argv,
+			    bool upgrade_exts)
 {
   gcc_assert (argc > 0 && argc <= 2);
   const char *default_arch_str = NULL;
@@ -1950,10 +2023,24 @@  riscv_expand_arch_from_cpu (int argc ATTRIBUTE_UNUSED,
   location_t loc = UNKNOWN_LOCATION;

   riscv_parse_arch_string (arch_str, NULL, loc);
-  const std::string arch = riscv_arch_str (false);
+  const std::string arch = riscv_arch_str (/*version_p*/ false, upgrade_exts);
   return xasprintf ("-march=%s", arch.c_str());
 }

+const char *
+riscv_expand_arch_from_cpu_upgrade_exts (int argc ATTRIBUTE_UNUSED,
+					 const char **argv)
+{
+  return riscv_expand_arch_from_cpu (argc, argv, /*upgrade_exts*/ true);
+}
+
+const char *
+riscv_expand_arch_from_cpu_no_upgrade_exts (int argc ATTRIBUTE_UNUSED,
+					    const char **argv)
+{
+  return riscv_expand_arch_from_cpu (argc, argv, /*upgrade_exts*/ false);
+}
+
 /* Report error if not found suitable multilib.  */
 const char *
 riscv_multi_lib_check (int argc ATTRIBUTE_UNUSED,
diff --git a/gcc/config.in b/gcc/config.in
index e41b6dc97cd..acab3c0f126 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -629,6 +629,12 @@ 
 #endif


+/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
+#endif
+
+
 /* Define if the assembler understands -march=rv*_zifencei. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_AS_MARCH_ZIFENCEI
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index d6473d0cd85..06b33c9f330 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -183,7 +183,8 @@  extern tree riscv_builtin_decl (unsigned int, bool);
 extern void riscv_init_builtins (void);

 /* Routines implemented in riscv-common.cc.  */
-extern std::string riscv_arch_str (bool version_p = true);
+extern std::string riscv_arch_str (bool version_p = true,
+				   bool upgrade_exts = false);
 extern void riscv_parse_arch_string (const char *, struct gcc_options *, location_t);

 extern bool riscv_hard_regno_rename_ok (unsigned, unsigned);
diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
index fe7f54d8bc5..8384aab63cb 100644
--- a/gcc/config/riscv/riscv-subset.h
+++ b/gcc/config/riscv/riscv-subset.h
@@ -90,7 +90,7 @@  public:
 			  int major_version = RISCV_DONT_CARE_VERSION,
 			  int minor_version = RISCV_DONT_CARE_VERSION) const;

-  std::string to_string (bool) const;
+  std::string to_string (bool, bool) const;

   unsigned xlen () const {return m_xlen;};

diff --git a/gcc/config/riscv/riscv-target-attr.cc b/gcc/config/riscv/riscv-target-attr.cc
index 1a73d69bf50..84c4d830918 100644
--- a/gcc/config/riscv/riscv-target-attr.cc
+++ b/gcc/config/riscv/riscv-target-attr.cc
@@ -367,7 +367,9 @@  riscv_process_target_attr (tree fndecl, tree args, location_t loc,
   /* Add the string of the target attribute to the fndecl hash table.  */
   riscv_subset_list *subset_list = attr_parser.get_riscv_subset_list ();
   if (subset_list)
-    riscv_func_target_put (fndecl, subset_list->to_string (true));
+    riscv_func_target_put (fndecl,
+			   subset_list->to_string (/*version_p*/ true,
+						   /*upgrade_exts*/ true));

   return true;
 }
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index c17141d909a..09943389986 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -9425,8 +9425,10 @@  riscv_sched_adjust_cost (rtx_insn *, int, rtx_insn *insn, int cost,
 static void
 riscv_emit_attribute ()
 {
+  /* Upgrade extensions if necessary for the assember to understand
+     Eg. Zalrsc -> a.  */
   fprintf (asm_out_file, "\t.attribute arch, \"%s\"\n",
-	   riscv_arch_str ().c_str ());
+	   riscv_arch_str (/*version_p*/ true, /*upgrade_exts*/ true).c_str ());

   fprintf (asm_out_file, "\t.attribute unaligned_access, %d\n",
            TARGET_STRICT_ALIGN ? 0 : 1);
@@ -9468,7 +9470,8 @@  riscv_declare_function_name (FILE *stream, const char *name, tree fndecl)
       std::string *target_name = riscv_func_target_get (fndecl);
       std::string isa = target_name != NULL
 	? *target_name
-	: riscv_cmdline_subset_list ()->to_string (true);
+	: riscv_cmdline_subset_list ()->to_string (/*version_p*/ true,
+						   /*upgrade_exts*/ true);
       fprintf (stream, "\t.option arch, %s\n", isa.c_str ());
       riscv_func_target_remove_and_destory (fndecl);

diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 57910eecd3e..55d0a842bf2 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -46,17 +46,24 @@  along with GCC; see the file COPYING3.  If not see
 #define RISCV_TUNE_STRING_DEFAULT "rocket"
 #endif

-extern const char *riscv_expand_arch (int argc, const char **argv);
-extern const char *riscv_expand_arch_from_cpu (int argc, const char **argv);
+extern const char *riscv_expand_arch_upgrade_exts (int argc, const char **argv);
+extern const char *riscv_expand_arch_no_upgrade_exts (int argc,
+						      const char **argv);
+extern const char *riscv_expand_arch_from_cpu_upgrade_exts (int argc,
+							    const char **argv);
+extern const char *riscv_expand_arch_from_cpu_no_upgrade_exts (int argc,
+							       const char **argv);
 extern const char *riscv_default_mtune (int argc, const char **argv);
 extern const char *riscv_multi_lib_check (int argc, const char **argv);
 extern const char *riscv_arch_help (int argc, const char **argv);

-# define EXTRA_SPEC_FUNCTIONS						\
-  { "riscv_expand_arch", riscv_expand_arch },				\
-  { "riscv_expand_arch_from_cpu", riscv_expand_arch_from_cpu },		\
-  { "riscv_default_mtune", riscv_default_mtune },			\
-  { "riscv_multi_lib_check", riscv_multi_lib_check },			\
+# define EXTRA_SPEC_FUNCTIONS							   \
+  { "riscv_expand_arch_u", riscv_expand_arch_upgrade_exts },			   \
+  { "riscv_expand_arch_nu", riscv_expand_arch_no_upgrade_exts },		   \
+  { "riscv_expand_arch_from_cpu_nu", riscv_expand_arch_from_cpu_no_upgrade_exts }, \
+  { "riscv_expand_arch_from_cpu_u", riscv_expand_arch_from_cpu_upgrade_exts },	   \
+  { "riscv_default_mtune", riscv_default_mtune },				   \
+  { "riscv_multi_lib_check", riscv_multi_lib_check },				   \
   { "riscv_arch_help", riscv_arch_help },

 /* Support for a compile-time default CPU, et cetera.  The rules are:
@@ -68,15 +75,15 @@  extern const char *riscv_arch_help (int argc, const char **argv);

    But using default -march/-mtune value if -mcpu don't have valid option.  */
 #define OPTION_DEFAULT_SPECS \
-  {"tune", "%{!mtune=*:"						\
-	   "  %{!mcpu=*:-mtune=%(VALUE)}"				\
-	   "  %{mcpu=*:-mtune=%:riscv_default_mtune(%* %(VALUE))}}" },	\
-  {"arch", "%{!march=*:"						\
-	   "  %{!mcpu=*:-march=%(VALUE)}"				\
-	   "  %{mcpu=*:%:riscv_expand_arch_from_cpu(%* %(VALUE))}}" },	\
-  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" },				\
-  {"isa_spec", "%{!misa-spec=*:-misa-spec=%(VALUE)}" },			\
-  {"tls", "%{!mtls-dialect=*:-mtls-dialect=%(VALUE)}"},         	\
+  {"tune", "%{!mtune=*:"						  \
+	   "  %{!mcpu=*:-mtune=%(VALUE)}"				  \
+	   "  %{mcpu=*:-mtune=%:riscv_default_mtune(%* %(VALUE))}}" },	  \
+  {"arch", "%{!march=*:"						  \
+	   "  %{!mcpu=*:-march=%(VALUE)}"				  \
+	   "  %{mcpu=*:%:riscv_expand_arch_from_cpu_nu(%* %(VALUE))}}" }, \
+  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" },				  \
+  {"isa_spec", "%{!misa-spec=*:-misa-spec=%(VALUE)}" },			  \
+  {"tls", "%{!mtls-dialect=*:-mtls-dialect=%(VALUE)}"},			  \

 #ifdef IN_LIBGCC2
 #undef TARGET_64BIT
@@ -103,7 +110,8 @@  extern const char *riscv_arch_help (int argc, const char **argv);
 #define ASM_SPEC "\
 %(subtarget_asm_debugging_spec) \
 %{" FPIE_OR_FPIC_SPEC ":-fpic} \
-%{march=*} \
+%{march=*:%:riscv_expand_arch_u(%*)} \
+%{!march=*:%{mcpu=*:%:riscv_expand_arch_from_cpu_u(%*)}} \
 %{mabi=*} \
 %{mno-relax} \
 %{mbig-endian} \
@@ -116,8 +124,8 @@  ASM_MISA_SPEC
 "%{march=help:%:riscv_arch_help()} "				\
 "%{print-supported-extensions:%:riscv_arch_help()} "		\
 "%{-print-supported-extensions:%:riscv_arch_help()} "		\
-"%{march=*:%:riscv_expand_arch(%*)} "				\
-"%{!march=*:%{mcpu=*:%:riscv_expand_arch_from_cpu(%*)}} "
+"%{march=*:%:riscv_expand_arch_nu(%*)} "			\
+"%{!march=*:%{mcpu=*:%:riscv_expand_arch_from_cpu_nu(%*)}} "

 #define TARGET_DEFAULT_CMODEL CM_MEDLOW

diff --git a/gcc/configure b/gcc/configure
index aaf5899cc03..b7fa67ef7dc 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -30820,6 +30820,37 @@  if test $gcc_cv_as_riscv_march_zifencei = yes; then

 $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h

+fi
+
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
+$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
+if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  gcc_cv_as_riscv_march_zaamo_zalrsc=no
+  if test x$gcc_cv_as != x; then
+    $as_echo '' > conftest.s
+    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }
+    then
+	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
+    else
+      echo "configure: failed program was" >&5
+      cat conftest.s >&5
+    fi
+    rm -f conftest.o conftest.s
+  fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
+$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
+if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
+
+$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
+
 fi

     ;;
diff --git a/gcc/configure.ac b/gcc/configure.ac
index f8d67efeb98..c54748cd9aa 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5452,6 +5452,11 @@  configured with --enable-newlib-nano-formatted-io.])
       [-march=rv32i_zifencei2p0],,,
       [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
 		 [Define if the assembler understands -march=rv*_zifencei.])])
+    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
+      gcc_cv_as_riscv_march_zaamo_zalrsc,
+      [-march=rv32i_zaamo_zalrsc],,,
+      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
+		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
     ;;
     loongarch*-*-*)
     gcc_GAS_CHECK_FEATURE([.dtprelword support],