[09/11] aarch64: Rewrite architecture strings for assembler
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
Add infrastructure to allow rewriting the architecture strings passed to
the assembler (either as -march options or .arch directives). There was
already canonicalisation everywhere except for an -march driver option
passed directly to the compiler; this patch applies the same
canonicalisation there as well.
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(aarch64_get_arch_string_for_assembler): New.
(aarch64_rewrite_march): New.
(aarch64_rewrite_selected_cpu): Call new function.
* config/aarch64/aarch64-elf.h (ASM_SPEC): Remove identity mapping.
* config/aarch64/aarch64-protos.h
(aarch64_get_arch_string_for_assembler): New.
* config/aarch64/aarch64.cc
(aarch64_declare_function_name): Call new function.
(aarch64_start_file): Ditto.
* config/aarch64/aarch64.h
* config/aarch64/aarch64.h
(EXTRA_SPEC_FUNCTIONS): Use new macro name.
(MCPU_TO_MARCH_SPEC): Rename to...
(MARCH_REWRITE_SPEC): ...this, and add new spec rule.
(aarch64_rewrite_march): New declaration.
(MCPU_TO_MARCH_SPEC_FUNCTIONS): Rename to...
(MARCH_REWRITE_SPEC_FUNCTIONS): ...this, and add new function.
(ASM_CPU_SPEC): Use new macro name.
Comments
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> Add infrastructure to allow rewriting the architecture strings passed to
> the assembler (either as -march options or .arch directives). There was
> already canonicalisation everywhere except for an -march driver option
> passed directly to the compiler; this patch applies the same
> canonicalisation there as well.
>
> gcc/ChangeLog:
>
> * common/config/aarch64/aarch64-common.cc
> (aarch64_get_arch_string_for_assembler): New.
> (aarch64_rewrite_march): New.
> (aarch64_rewrite_selected_cpu): Call new function.
> * config/aarch64/aarch64-elf.h (ASM_SPEC): Remove identity mapping.
> * config/aarch64/aarch64-protos.h
> (aarch64_get_arch_string_for_assembler): New.
> * config/aarch64/aarch64.cc
> (aarch64_declare_function_name): Call new function.
> (aarch64_start_file): Ditto.
> * config/aarch64/aarch64.h
> * config/aarch64/aarch64.h
> (EXTRA_SPEC_FUNCTIONS): Use new macro name.
> (MCPU_TO_MARCH_SPEC): Rename to...
> (MARCH_REWRITE_SPEC): ...this, and add new spec rule.
> (aarch64_rewrite_march): New declaration.
> (MCPU_TO_MARCH_SPEC_FUNCTIONS): Rename to...
> (MARCH_REWRITE_SPEC_FUNCTIONS): ...this, and add new function.
> (ASM_CPU_SPEC): Use new macro name.
Looks good, but it'll need to be rebased on top of Tamar's fix to
MARCH_REWRITE_SPEC (please wait for that to go in first). On that:
> @@ -1502,18 +1502,21 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
> {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
> CONFIG_TUNE_SPEC
>
> -#define MCPU_TO_MARCH_SPEC \
> - " %{mcpu=*:-march=%:rewrite_mcpu(%{mcpu=*:%*})}"
> +#define MARCH_REWRITE_SPEC \
> + " %{mcpu=*:-march=%:rewrite_mcpu(%{mcpu=*:%*})}" \
> + " %{march=*:-march=%:rewrite_march(%{march=*:%*})}"
I suppose the way of handling both Tamar's change and yours would be
something like:
"%{march=*:-march=%:rewrite_march(%{march=*:%*})" \
";:%{mcpu=*:-march=%:rewrite_mcpu(%{mcpu=*:%*})}}"
(untested).
> +extern const char *aarch64_rewrite_march (int argc, const char **argv);
> extern const char *aarch64_rewrite_mcpu (int argc, const char **argv);
> extern const char *is_host_cpu_not_armv8_base (int argc, const char **argv);
> -#define MCPU_TO_MARCH_SPEC_FUNCTIONS \
> +#define MARCH_REWRITE_SPEC_FUNCTIONS \
MARCH_REWRITE_SPEC_FUNCTIONS also doesn't quite cover it. How about
just AARCH64_BASE_SPEC_FUNCTIONS, since its purpose is to define the
functions needed for both cross and native hosts?
OK with those changes if you agree.
Thanks,
Richard
> + { "rewrite_march", aarch64_rewrite_march }, \
> { "rewrite_mcpu", aarch64_rewrite_mcpu }, \
> { "is_local_not_armv8_base", is_host_cpu_not_armv8_base },
>
>
> #define ASM_CPU_SPEC \
> - MCPU_TO_MARCH_SPEC
> + MARCH_REWRITE_SPEC
>
> #define EXTRA_SPECS \
> { "asm_cpu_spec", ASM_CPU_SPEC }
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 75ba66a979c979fd01948b0a2066a15371df9bfa..95861c1088052cc60d1e02c654ee970cb8bc3bef 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -24831,16 +24831,12 @@ aarch64_declare_function_name (FILE *stream, const char* name,
> targ_options = TREE_TARGET_OPTION (target_option_current_node);
> gcc_assert (targ_options);
>
> - const struct processor *this_arch
> - = aarch64_get_arch (targ_options->x_selected_arch);
> -
> auto isa_flags = aarch64_get_asm_isa_flags (targ_options);
> - std::string extension
> - = aarch64_get_extension_string_for_isa_flags (isa_flags,
> - this_arch->flags);
> + aarch64_arch arch = targ_options->x_selected_arch;
> + std::string to_print
> + = aarch64_get_arch_string_for_assembler (arch, isa_flags);
> /* Only update the assembler .arch string if it is distinct from the last
> such string we printed. */
> - std::string to_print = this_arch->name + extension;
> if (to_print != aarch64_last_printed_arch_string)
> {
> asm_fprintf (asm_out_file, "\t.arch %s\n", to_print.c_str ());
> @@ -24962,19 +24958,16 @@ aarch64_start_file (void)
> struct cl_target_option *default_options
> = TREE_TARGET_OPTION (target_option_default_node);
>
> - const struct processor *default_arch
> - = aarch64_get_arch (default_options->x_selected_arch);
> + aarch64_arch default_arch = default_options->x_selected_arch;
> auto default_isa_flags = aarch64_get_asm_isa_flags (default_options);
> - std::string extension
> - = aarch64_get_extension_string_for_isa_flags (default_isa_flags,
> - default_arch->flags);
> -
> - aarch64_last_printed_arch_string = default_arch->name + extension;
> - aarch64_last_printed_tune_string = "";
> - asm_fprintf (asm_out_file, "\t.arch %s\n",
> - aarch64_last_printed_arch_string.c_str ());
> -
> - default_file_start ();
> + std::string arch_string
> + = aarch64_get_arch_string_for_assembler (default_arch, default_isa_flags);
> + aarch64_last_printed_arch_string = arch_string;
> + aarch64_last_printed_tune_string = "";
> + asm_fprintf (asm_out_file, "\t.arch %s\n",
> + arch_string.c_str ());
> +
> + default_file_start ();
> }
>
> /* Emit load exclusive. */
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> @@ -697,6 +697,50 @@ aarch64_get_extension_string_for_isa_flags
> + const struct arch_info *entry;
> + for (entry = all_architectures; entry->arch != aarch64_no_arch; entry++)
> + {
> + if (entry->arch == arch)
> + break;
> + }
Sorry for the nit, but forgot to say: the convention is not to have
braces here.
Thanks,
Richard
@@ -697,6 +697,50 @@ aarch64_get_extension_string_for_isa_flags
return outstr;
}
+/* Generate an arch string to be passed to the assembler. */
+
+std::string
+aarch64_get_arch_string_for_assembler (aarch64_arch arch,
+ aarch64_feature_flags flags)
+{
+ const struct arch_info *entry;
+ for (entry = all_architectures; entry->arch != aarch64_no_arch; entry++)
+ {
+ if (entry->arch == arch)
+ break;
+ }
+
+ std::string outstr = entry->name
+ + aarch64_get_extension_string_for_isa_flags (flags, entry->flags);
+
+ return outstr;
+}
+
+/* Called by the driver to rewrite a name passed to the -march
+ argument in preparation to be passed to the assembler. The
+ names passed from the commend line will be in ARGV, we want
+ to use the right-most argument, which should be in
+ ARGV[ARGC - 1]. ARGC should always be greater than 0. */
+
+const char *
+aarch64_rewrite_march (int argc, const char **argv)
+{
+ gcc_assert (argc);
+ const char *name = argv[argc - 1];
+ aarch64_arch arch;
+ aarch64_feature_flags flags;
+
+ aarch64_validate_march (name, &arch, &flags);
+
+ std::string outstr = aarch64_get_arch_string_for_assembler (arch, flags);
+
+ /* We are going to memory leak here, nobody elsewhere
+ in the callchain is going to clean up after us. The alternative is
+ to allocate a static buffer, and assert that it is big enough for our
+ modified string, which seems much worse! */
+ return xstrdup (outstr.c_str ());
+}
+
/* Attempt to rewrite NAME, which has been passed on the command line
as a -mcpu option to an equivalent -march value. If we can do so,
return the new string, otherwise return an error. */
@@ -740,7 +784,7 @@ aarch64_rewrite_selected_cpu (const char *name)
break;
}
- /* We couldn't find that proceesor name, or the processor name we
+ /* We couldn't find that processor name, or the processor name we
found does not map to an architecture we understand. */
if (p_to_a->arch == aarch64_no_arch
|| a_to_an->arch == aarch64_no_arch)
@@ -749,9 +793,8 @@ aarch64_rewrite_selected_cpu (const char *name)
aarch64_feature_flags extensions = p_to_a->flags;
aarch64_parse_extension (extension_str.c_str (), &extensions, NULL);
- std::string outstr = a_to_an->name
- + aarch64_get_extension_string_for_isa_flags (extensions,
- a_to_an->flags);
+ std::string outstr = aarch64_get_arch_string_for_assembler (a_to_an->arch,
+ extensions);
/* We are going to memory leak here, nobody elsewhere
in the callchain is going to clean up after us. The alternative is
@@ -136,7 +136,6 @@
#define ASM_SPEC "\
%{mbig-endian:-EB} \
%{mlittle-endian:-EL} \
-%{march=*:-march=%*} \
%(asm_cpu_spec)" \
ASM_MABI_SPEC
#endif
@@ -1213,6 +1213,8 @@ bool aarch64_validate_mtune (const char *, aarch64_cpu *);
const char *aarch64_rewrite_selected_cpu (const char *name);
std::string aarch64_get_extension_string_for_isa_flags (aarch64_feature_flags,
aarch64_feature_flags);
+std::string aarch64_get_arch_string_for_assembler (aarch64_arch,
+ aarch64_feature_flags);
rtl_opt_pass *make_pass_aarch64_early_ra (gcc::context *);
rtl_opt_pass *make_pass_fma_steering (gcc::context *);
@@ -1470,7 +1470,7 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
#define HAVE_LOCAL_CPU_DETECT
# define EXTRA_SPEC_FUNCTIONS \
{ "local_cpu_detect", host_detect_local_cpu }, \
- MCPU_TO_MARCH_SPEC_FUNCTIONS
+ MARCH_REWRITE_SPEC_FUNCTIONS
/* Rewrite -m{arch,cpu,tune}=native based on the host system information.
When rewriting -march=native convert it into an -mcpu option if no other
@@ -1487,7 +1487,7 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
{ "tune", "%{!mcpu=*:%{!mtune=*:%{!march=native:-mtune=%(VALUE)}}}" },
#else
# define MCPU_MTUNE_NATIVE_SPECS ""
-# define EXTRA_SPEC_FUNCTIONS MCPU_TO_MARCH_SPEC_FUNCTIONS
+# define EXTRA_SPEC_FUNCTIONS MARCH_REWRITE_SPEC_FUNCTIONS
# define CONFIG_TUNE_SPEC \
{"tune", "%{!mcpu=*:%{!mtune=*:-mtune=%(VALUE)}}"},
#endif
@@ -1502,18 +1502,21 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
{"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
CONFIG_TUNE_SPEC
-#define MCPU_TO_MARCH_SPEC \
- " %{mcpu=*:-march=%:rewrite_mcpu(%{mcpu=*:%*})}"
+#define MARCH_REWRITE_SPEC \
+ " %{mcpu=*:-march=%:rewrite_mcpu(%{mcpu=*:%*})}" \
+ " %{march=*:-march=%:rewrite_march(%{march=*:%*})}"
+extern const char *aarch64_rewrite_march (int argc, const char **argv);
extern const char *aarch64_rewrite_mcpu (int argc, const char **argv);
extern const char *is_host_cpu_not_armv8_base (int argc, const char **argv);
-#define MCPU_TO_MARCH_SPEC_FUNCTIONS \
+#define MARCH_REWRITE_SPEC_FUNCTIONS \
+ { "rewrite_march", aarch64_rewrite_march }, \
{ "rewrite_mcpu", aarch64_rewrite_mcpu }, \
{ "is_local_not_armv8_base", is_host_cpu_not_armv8_base },
#define ASM_CPU_SPEC \
- MCPU_TO_MARCH_SPEC
+ MARCH_REWRITE_SPEC
#define EXTRA_SPECS \
{ "asm_cpu_spec", ASM_CPU_SPEC }
@@ -24831,16 +24831,12 @@ aarch64_declare_function_name (FILE *stream, const char* name,
targ_options = TREE_TARGET_OPTION (target_option_current_node);
gcc_assert (targ_options);
- const struct processor *this_arch
- = aarch64_get_arch (targ_options->x_selected_arch);
-
auto isa_flags = aarch64_get_asm_isa_flags (targ_options);
- std::string extension
- = aarch64_get_extension_string_for_isa_flags (isa_flags,
- this_arch->flags);
+ aarch64_arch arch = targ_options->x_selected_arch;
+ std::string to_print
+ = aarch64_get_arch_string_for_assembler (arch, isa_flags);
/* Only update the assembler .arch string if it is distinct from the last
such string we printed. */
- std::string to_print = this_arch->name + extension;
if (to_print != aarch64_last_printed_arch_string)
{
asm_fprintf (asm_out_file, "\t.arch %s\n", to_print.c_str ());
@@ -24962,19 +24958,16 @@ aarch64_start_file (void)
struct cl_target_option *default_options
= TREE_TARGET_OPTION (target_option_default_node);
- const struct processor *default_arch
- = aarch64_get_arch (default_options->x_selected_arch);
+ aarch64_arch default_arch = default_options->x_selected_arch;
auto default_isa_flags = aarch64_get_asm_isa_flags (default_options);
- std::string extension
- = aarch64_get_extension_string_for_isa_flags (default_isa_flags,
- default_arch->flags);
-
- aarch64_last_printed_arch_string = default_arch->name + extension;
- aarch64_last_printed_tune_string = "";
- asm_fprintf (asm_out_file, "\t.arch %s\n",
- aarch64_last_printed_arch_string.c_str ());
-
- default_file_start ();
+ std::string arch_string
+ = aarch64_get_arch_string_for_assembler (default_arch, default_isa_flags);
+ aarch64_last_printed_arch_string = arch_string;
+ aarch64_last_printed_tune_string = "";
+ asm_fprintf (asm_out_file, "\t.arch %s\n",
+ arch_string.c_str ());
+
+ default_file_start ();
}
/* Emit load exclusive. */