* [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
* Re: [RFC v3] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils
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
0 siblings, 1 reply; 5+ messages in thread
From: Kito Cheng @ 2024-06-18 0:51 UTC (permalink / raw)
To: Patrick O'Neill
Cc: gcc-patches, palmer, jeffreyalaw, pan2.li, gnu-toolchain
Maybe just add 'a' to riscv_combine_info and other logic to keep the
same (e.g. keep the logic for skip_zaamo_zalrsc)?
On Tue, Jun 18, 2024 at 8:03 AM Patrick O'Neill <patrick@rivosinc.com> wrote:
>
> 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
* Re: [RFC v3] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils
2024-06-18 0:51 ` Kito Cheng
@ 2024-06-18 5:34 ` Patrick O'Neill
2024-06-18 5:45 ` Kito Cheng
0 siblings, 1 reply; 5+ messages in thread
From: Patrick O'Neill @ 2024-06-18 5:34 UTC (permalink / raw)
To: Kito Cheng; +Cc: gcc-patches, gnu-toolchain, jeffreyalaw, palmer, pan2.li
[-- Attachment #1: Type: text/plain, Size: 19931 bytes --]
On Mon, Jun 17, 2024 at 5:51 PM Kito Cheng <kito.cheng@gmail.com> wrote:
> Maybe just add 'a' to riscv_combine_info and other logic to keep the
> same (e.g. keep the logic for skip_zaamo_zalrsc)?
I did consider unconditionally upgrading zaamo/zalrsc to ‘a’ (I think
that’s what you’re suggesting w/ riscv_combine_info).
That could cause issues if users are trying to compile for a zalrsc-only
chip with an old version of binutils. If we upgrade zalrsc -> ‘a’ for both
cc1 and binutils then cc1 will emit amo ops instead of their lr/sc
equivalent.
GCC would end up emitting insns that are illegal for the user-provided
-march string.
Patrick
> On Tue, Jun 18, 2024 at 8:03 AM Patrick O'Neill <patrick@rivosinc.com>
> wrote:
> >
> > 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
* Re: [RFC v3] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils
2024-06-18 5:34 ` Patrick O'Neill
@ 2024-06-18 5:45 ` Kito Cheng
2024-06-18 16:32 ` Patrick O'Neill
0 siblings, 1 reply; 5+ messages in thread
From: Kito Cheng @ 2024-06-18 5:45 UTC (permalink / raw)
To: Patrick O'Neill
Cc: gcc-patches, gnu-toolchain, jeffreyalaw, palmer, pan2.li
When 'a' is put into riscv_combine_info, 'a' will only be added into
arch string only if zaamo *AND* zalrsc is there, so zalrsc only won't
trigger that.
On Tue, Jun 18, 2024 at 1:35 PM Patrick O'Neill <patrick@rivosinc.com> wrote:
>
>
>
> On Mon, Jun 17, 2024 at 5:51 PM Kito Cheng <kito.cheng@gmail.com> wrote:
>>
>> Maybe just add 'a' to riscv_combine_info and other logic to keep the
>> same (e.g. keep the logic for skip_zaamo_zalrsc)?
>
>
> I did consider unconditionally upgrading zaamo/zalrsc to ‘a’ (I think that’s what you’re suggesting w/ riscv_combine_info).
> That could cause issues if users are trying to compile for a zalrsc-only chip with an old version of binutils. If we upgrade zalrsc -> ‘a’ for both cc1 and binutils then cc1 will emit amo ops instead of their lr/sc equivalent.
> GCC would end up emitting insns that are illegal for the user-provided -march string.
>
> Patrick
>
>>
>> On Tue, Jun 18, 2024 at 8:03 AM Patrick O'Neill <patrick@rivosinc.com> wrote:
>> >
>> > 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
* Re: [RFC v3] RISC-V: Promote Zaamo/Zalrsc to a when using an old binutils
2024-06-18 5:45 ` Kito Cheng
@ 2024-06-18 16:32 ` Patrick O'Neill
0 siblings, 0 replies; 5+ messages in thread
From: Patrick O'Neill @ 2024-06-18 16:32 UTC (permalink / raw)
To: Kito Cheng; +Cc: gcc-patches, gnu-toolchain, jeffreyalaw, palmer, pan2.li
Ah that makes sense. We discussed it a bit during the patchworks
meeting - I'll drop the other changes and add it to riscv_combine_info.
Thanks,
Patrick
On 6/17/24 22:45, Kito Cheng wrote:
> When 'a' is put into riscv_combine_info, 'a' will only be added into
> arch string only if zaamo *AND* zalrsc is there, so zalrsc only won't
> trigger that.
>
> On Tue, Jun 18, 2024 at 1:35 PM Patrick O'Neill <patrick@rivosinc.com> wrote:
>>
>>
>> On Mon, Jun 17, 2024 at 5:51 PM Kito Cheng <kito.cheng@gmail.com> wrote:
>>> Maybe just add 'a' to riscv_combine_info and other logic to keep the
>>> same (e.g. keep the logic for skip_zaamo_zalrsc)?
>>
>> I did consider unconditionally upgrading zaamo/zalrsc to ‘a’ (I think that’s what you’re suggesting w/ riscv_combine_info).
>> That could cause issues if users are trying to compile for a zalrsc-only chip with an old version of binutils. If we upgrade zalrsc -> ‘a’ for both cc1 and binutils then cc1 will emit amo ops instead of their lr/sc equivalent.
>> GCC would end up emitting insns that are illegal for the user-provided -march string.
>>
>> Patrick
>>
>>> On Tue, Jun 18, 2024 at 8:03 AM Patrick O'Neill <patrick@rivosinc.com> wrote:
>>>> 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).