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 <patr...@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 <patr...@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 >