On 21/07/15 12:09, James Greenhalgh wrote:
On Fri, Jul 17, 2015 at 03:19:20PM +0100, Kyrill Tkachov wrote:
This is a slight respin of this patch, handling the -moverride string more
gracefully.
We need to explicitly save and restore it in TARGET_OPTION_SAVE otherwise the
option gen machinery
gets confused about its type and during its printing uses the wrong format code
for the pointer, leading to a warning that may trigger during bootstrap.
Otherwise it is the same as the previous version.
Bootstrapped and tested on aarch64.
I'd like to propose this version instead of the original.
Ok?
The ChangeLog looks outdated with respect to the patch, in particular:
(x_aarch64_isa_flags): Likewise.
This is just "aarch64_isa_flags" in the file.
+TargetSave
+const char *x_aarch64_override_tune_string
+
This is not mentioned.
(master=): Likewise.
This doesn't exist.
+#undef TARGET_OPTION_SAVE
+#define TARGET_OPTION_SAVE aarch64_option_save
+
This is not mentioned.
In addition to the ChangeLog nits,
Oops, thanks for catching those, here's an updated one.
2015-07-21 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
* config/aarch64/aarch64.opt (explicit_tune_core): New TargetVariable.
(explicit_arch): Likewise.
(aarch64_isa_flags): Likewise.
(x_aarch64_override_tune_string): Likewise.
(mgeneral-regs-only): Mark as Save.
(mfix-cortex-a53-835769): Likewise.
(mcmodel=): Likewise.
(mstrict-align): Likewise.
(momit-leaf-frame-pointer): Likewise.
(mtls-dialect): Likewise.
* config/aarch64/aarch64.h (ASM_DECLARE_FUNCTION_NAME): Define.
(aarch64_isa_flags): Remove extern declaration.
* config/aarch64/aarch64.c (aarch64_validate_mcpu): Return a bool
to indicate success or failure.
(aarch64_validate_march): Likewise.
(aarch64_validate_mtune): Likewise.
(aarch64_isa_flags): Delete.
(aarch64_override_options_internal): Access opts->x_aarch64_isa_flags
instead of aarch64_isa_flags.
(aarch64_get_tune_cpu): New function.
(aarch64_get_arch): Likewise.
(aarch64_override_options): Use above and set up explicit_tune_core
and explicit_arch.
(aarch64_print_extension): Move earlier in file. Add isa_flags
argument and use that instead of the global aarch64_isa_flags.
(aarch64_option_restore): Likewise.
(aarch64_option_print): Likewise.
(aarch64_declare_function_name): Likewise.
(aarch64_start_file): Delete.
(TARGET_ASM_FILE_START): Do not define.
(TARGET_OPTION_SAVE, TARGET_OPTION_RESTORE, TARGET_OPTION_PRINT):
Define.
* config/aarch64/aarch64-protos.h (aarch64_declare_function_name):
Declare prototype.
+/* Return the CPU corresponding to the enum CPU.
+ If it doesn't specify a cpu, return the default. */
+
+static const struct processor *
+aarch64_get_tune_cpu (enum aarch64_processor cpu)
+{
+ if (cpu != aarch64_none)
+ return &all_cores[cpu];
+
+ return &all_cores[TARGET_CPU_DEFAULT & 0x3f];
+}
This looks strange to me, is there nothing we can do to make it clear
what "0x3f" means in this context, or better yet to get rid of the
two-in-one variable...
This is due to the cryptic way in which we specify the configure-time
cpu and arch extensions. Perhaps a comment at the default definition of
TARGET_CPU_DEFAULT in aarch64.h would partially mitigate the confusion.
Thanks,
Kyrill
OK with a fixed ChangeLog.
Thanks,
James
