public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC v3] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils
@ 2024-06-18  0:02 Patrick O'Neill
  2024-06-18  0:51 ` Kito Cheng
  0 siblings, 1 reply; 5+ messages in thread
From: Patrick O'Neill @ 2024-06-18  0:02 UTC (permalink / raw)
  To: gcc-patches
  Cc: palmer, jeffreyalaw, kito.cheng, pan2.li, gnu-toolchain,
	Patrick O'Neill

Binutils 2.42 and before don't support Zaamo/Zalrsc. Promote Zaamo/Zalrsc to
'a' in the -march string when assembling.

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.

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
v3 ChangeLog:
Rebased on non-promoting patch.
Wrap all Zaamo/Zalrsc upgrade code in #ifndef to prevent compiler
warnings about unused/potentially undefined variables.
Silence unused parameter warning with a voidcast.
---
RFC since I'm not sure if this upgrade behavior is more trouble than
it's worth - this is a pretty invasive change. Happy to iterate further
or just drop these changes.
---
 gcc/common/config/riscv/riscv-common.cc | 111 +++++++++++++++++++++---
 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 ++++++----
 6 files changed, 137 insertions(+), 36 deletions(-)

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 1dc1d9904c7..05c26f73b73 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,10 +916,17 @@ riscv_subset_list::to_string (bool version_p) const
   riscv_subset_t *subset;

   bool skip_zifencei = false;
-  bool skip_zaamo_zalrsc = false;
   bool skip_zicsr = false;
   bool i2p0 = false;

+#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
+  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;
+#endif
+
   /* For RISC-V ISA version 2.2 or earlier version, zicsr and zifencei is
      included in the base ISA.  */
   if (riscv_isa_spec == ISA_SPEC_CLASS_2P2)
@@ -945,8 +952,33 @@ riscv_subset_list::to_string (bool version_p) const
   skip_zifencei = true;
 #endif
 #ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
-  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
-  skip_zaamo_zalrsc = true;
+  /* 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;
+	}
+    }
+#else
+  /* Silence unused parameter warning when HAVE_AS_MARCH_ZAAMO_ZALRSC is
+     set.  */
+  (void) upgrade_exts;
 #endif

   for (subset = m_head; subset != NULL; subset = subset->next)
@@ -959,12 +991,29 @@ riscv_subset_list::to_string (bool version_p) const
 	  subset->name == "zicsr")
 	continue;

-      if (skip_zaamo_zalrsc && subset->name == "zaamo")
+#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
+      if (upgrade_zaamo_zalrsc && subset->name == "zaamo")
 	continue;

-      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
+      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;
+	    }
+	}
+#endif
+
       /* For !version_p, we only separate extension with underline for
 	 multi-letter extension.  */
       if (!first &&
@@ -984,6 +1033,14 @@ riscv_subset_list::to_string (bool version_p) const
 	     << subset->minor_version;
     }

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

@@ -1598,10 +1655,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();
 }
@@ -1907,18 +1964,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.  */

@@ -1938,7 +2010,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;
@@ -1961,10 +2034,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/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 19eb7b06d54..42c2b439e66 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

--
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-18 16:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-18  0:02 [RFC v3] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils Patrick O'Neill
2024-06-18  0:51 ` Kito Cheng
2024-06-18  5:34   ` Patrick O'Neill
2024-06-18  5:45     ` Kito Cheng
2024-06-18 16:32       ` Patrick O'Neill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).