[PATCH][AArch64][3/14] Refactor option override code

[PATCH][AArch64][3/14] Refactor option override code

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 2493 bytes --]

Hi all,

This one is more meaty than the previous ones. It buffs up the parsing functions for
the mcpu, march, mtune options, decouples them and makes them return an enum describing
the errors that may occur.  This will allow us to use these functions in other contexts
beyond aarch64_override_options.

aarch64_override_options itself gets an overhaul and is split up into code that must run
only once after the command line option have been processed, and code that has to be run
every time the backend-specific state changes (after SWITCHABLE_TARGET is implemented).

The stuff that must be run every time the backend state changes is put into
aarch64_override_options_internal.

Also, this patch deletes the declarations of aarch64_{arch,cpu,tune}_string from aarch64.opt
as they are superfluous since the march, mtune and mcpu option specification implicitly
declares these variables.

This patch looks large, but a lot of it is moving code around...

Bootstrapped and tested as part of the series on aarch64.

Ok for trunk?

Thanks,
Kyrill

2015-07-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.opt (aarch64_arch_string): Delete.
     (aarch64_cpu_string): Likewise.
     (aarch64_tune_string): Likewise.
     * config/aarch64/aarch64.c (aarch64_parse_opt_result): New enum.
     (aarch64_parse_extension): Return aarch64_parse_opt_result.
     Add extra argument to put result into.
     (aarch64_parse_arch): Likewise.  Do not set selected_cpu.
     (aarch64_parse_cpu): Add arguments to put results into. Return
     aarch64_parse_opt_result.
     (aarch64_parse_tune): Likewise.
     (aarch64_override_options_after_change_1): New function.
     (aarch64_override_options_internal): New function.
     (aarch64_validate_mcpu): Likewise.
     (aarch64_validate_march): Likewise.
     (aarch64_validate_mtune): Likewise.
     (aarch64_override_options): Update to reflect above changes.
     Move some logic into aarch64_override_options_internal.
     Initialize target_option_default_node and target_option_current_node.
     (aarch64_override_options_after_change): Move logic into
     aarch64_override_options_after_change_1 and call it with global_options.
     (initialize_aarch64_code_model): Take a gcc_options pointer and use the
     flag values from that.

2015-07-06  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/aarch64/cpu-diagnostics-3.c: Update expected error string.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-attrs-3.patch --]
[-- Type: text/x-patch; name=aarch64-attrs-3.patch, Size: 21236 bytes --]

commit cea498a32becf7cd947864ff908f25fe447c89d2
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed May 6 16:27:00 2015 +0100

    [AArch64][3/N] Refactor option override code

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 32b974a..5ea65e3 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7101,12 +7101,27 @@ aarch64_add_stmt_cost (void *data, int count, enum vect_cost_for_stmt kind,
   return retval;
 }
 
-static void initialize_aarch64_code_model (void);
+static void initialize_aarch64_code_model (struct gcc_options *);
 
-/* Parse the architecture extension string.  */
+/* Enum describing the various ways that the
+   aarch64_parse_{arch,tune,cpu,extension} functions can fail.
+   This way their callers can choose what kind of error to give.  */
 
-static void
-aarch64_parse_extension (char *str)
+enum aarch64_parse_opt_result
+{
+  AARCH64_PARSE_OK,			/* Parsing was successful.  */
+  AARCH64_PARSE_MISSING_ARG,		/* Missing argument.  */
+  AARCH64_PARSE_INVALID_FEATURE,	/* Invalid feature modifier.  */
+  AARCH64_PARSE_INVALID_ARG		/* Invalid arch, tune, cpu arg.  */
+};
+
+
+/* Parse the architecture extension string STR and update ISA_FLAGS
+   with the architecture features turned on or off.  Return a
+   aarch64_parse_opt_result describing the result.  */
+
+static enum aarch64_parse_opt_result
+aarch64_parse_extension (char *str, unsigned long *isa_flags)
 {
   /* The extension string is parsed left to right.  */
   const struct aarch64_option_extension *opt = NULL;
@@ -7137,11 +7152,8 @@ aarch64_parse_extension (char *str)
 	adding_ext = 1;
 
       if (len == 0)
-	{
-	  error ("missing feature modifier after %qs", adding_ext ? "+"
-	                                                          : "+no");
-	  return;
-	}
+	return AARCH64_PARSE_MISSING_ARG;
+
 
       /* Scan over the extensions table trying to find an exact match.  */
       for (opt = all_extensions; opt->name != NULL; opt++)
@@ -7150,9 +7162,9 @@ aarch64_parse_extension (char *str)
 	    {
 	      /* Add or remove the extension.  */
 	      if (adding_ext)
-		aarch64_isa_flags |= opt->flags_on;
+		*isa_flags |= opt->flags_on;
 	      else
-		aarch64_isa_flags &= ~(opt->flags_off);
+		*isa_flags &= ~(opt->flags_off);
 	      break;
 	    }
 	}
@@ -7160,27 +7172,30 @@ aarch64_parse_extension (char *str)
       if (opt->name == NULL)
 	{
 	  /* Extension not found in list.  */
-	  error ("unknown feature modifier %qs", str);
-	  return;
+	  return AARCH64_PARSE_INVALID_FEATURE;
 	}
 
       str = ext;
     };
 
-  return;
+  return AARCH64_PARSE_OK;
 }
 
-/* Parse the ARCH string.  */
+/* Parse the TO_PARSE string and put the architecture struct that it
+   selects into RES and the architectural features into ISA_FLAGS.
+   Return an aarch64_parse_opt_result describing the parse result.
+   If there is an error parsing, RES and ISA_FLAGS are left unchanged.  */
 
-static void
-aarch64_parse_arch (void)
+static enum aarch64_parse_opt_result
+aarch64_parse_arch (const char *to_parse, const struct processor **res,
+		    unsigned long *isa_flags)
 {
   char *ext;
   const struct processor *arch;
-  char *str = (char *) alloca (strlen (aarch64_arch_string) + 1);
+  char *str = xstrdup (to_parse);
   size_t len;
 
-  strcpy (str, aarch64_arch_string);
+  strcpy (str, to_parse);
 
   ext = strchr (str, '+');
 
@@ -7191,54 +7206,59 @@ aarch64_parse_arch (void)
 
   if (len == 0)
     {
-      error ("missing arch name in -march=%qs", str);
-      return;
+      free (str);
+      return AARCH64_PARSE_MISSING_ARG;
     }
 
-  /* Loop through the list of supported ARCHs to find a match.  */
+
+  /* Loop through the list of supported ARCHes to find a match.  */
   for (arch = all_architectures; arch->name != NULL; arch++)
     {
       if (strlen (arch->name) == len && strncmp (arch->name, str, len) == 0)
 	{
-	  selected_arch = arch;
-	  aarch64_isa_flags = selected_arch->flags;
-
-	  if (!selected_cpu)
-	    selected_cpu = &all_cores[selected_arch->ident];
+	  unsigned long isa_temp = arch->flags;
 
 	  if (ext != NULL)
 	    {
-	      /* ARCH string contains at least one extension.  */
-	      aarch64_parse_extension (ext);
-	    }
+	      /* TO_PARSE string contains at least one extension.  */
+	      enum aarch64_parse_opt_result ext_res
+		= aarch64_parse_extension (ext, &isa_temp);
 
-	  if (selected_arch->arch != selected_cpu->arch)
-	    {
-	      warning (0, "switch -mcpu=%s conflicts with -march=%s switch",
-		       all_architectures[selected_cpu->arch].name,
-		       selected_arch->name);
+	      if (ext_res != AARCH64_PARSE_OK)
+		{
+		  free (str);
+		  return ext_res;
+		}
 	    }
-
-	  return;
+	  /* Extension parsing was successful.  Confirm the result
+	     arch and ISA flags.  */
+	  *res = arch;
+	  *isa_flags = isa_temp;
+	  free (str);
+	  return AARCH64_PARSE_OK;
 	}
     }
 
+  free (str);
   /* ARCH name not found in list.  */
-  error ("unknown value %qs for -march", str);
-  return;
+  return AARCH64_PARSE_INVALID_ARG;
 }
 
-/* Parse the CPU string.  */
+/* Parse the TO_PARSE string and put the result tuning in RES and the
+   architecture flags in ISA_FLAGS.  Return an aarch64_parse_opt_result
+   describing the parse result.  If there is an error parsing, RES and
+   ISA_FLAGS are left unchanged.  */
 
-static void
-aarch64_parse_cpu (void)
+static enum aarch64_parse_opt_result
+aarch64_parse_cpu (const char *to_parse, const struct processor **res,
+		   unsigned long *isa_flags)
 {
   char *ext;
   const struct processor *cpu;
-  char *str = (char *) alloca (strlen (aarch64_cpu_string) + 1);
+  char *str = xstrdup (to_parse);
   size_t len;
 
-  strcpy (str, aarch64_cpu_string);
+  strcpy (str, to_parse);
 
   ext = strchr (str, '+');
 
@@ -7249,8 +7269,8 @@ aarch64_parse_cpu (void)
 
   if (len == 0)
     {
-      error ("missing cpu name in -mcpu=%qs", str);
-      return;
+      free (str);
+      return AARCH64_PARSE_MISSING_ARG;
     }
 
   /* Loop through the list of supported CPUs to find a match.  */
@@ -7258,46 +7278,60 @@ aarch64_parse_cpu (void)
     {
       if (strlen (cpu->name) == len && strncmp (cpu->name, str, len) == 0)
 	{
-	  selected_cpu = cpu;
-	  aarch64_isa_flags = selected_cpu->flags;
+	  unsigned long isa_temp = cpu->flags;
+
 
 	  if (ext != NULL)
 	    {
-	      /* CPU string contains at least one extension.  */
-	      aarch64_parse_extension (ext);
-	    }
+	      /* TO_PARSE string contains at least one extension.  */
+	      enum aarch64_parse_opt_result ext_res
+		= aarch64_parse_extension (ext, &isa_temp);
 
-	  return;
+	      if (ext_res != AARCH64_PARSE_OK)
+		{
+		  free (str);
+		  return ext_res;
+		}
+	    }
+	  /* Extension parsing was successfull.  Confirm the result
+	     cpu and ISA flags.  */
+	  *res = cpu;
+	  *isa_flags = isa_temp;
+	  free (str);
+	  return AARCH64_PARSE_OK;
 	}
     }
 
+  free (str);
   /* CPU name not found in list.  */
-  error ("unknown value %qs for -mcpu", str);
-  return;
+  return AARCH64_PARSE_INVALID_ARG;
 }
 
-/* Parse the TUNE string.  */
+/* Parse the TO_PARSE string and put the cpu it selects into RES.
+   Return an aarch64_parse_opt_result describing the parse result.
+   If the parsing fails the RES does not change.  */
 
-static void
-aarch64_parse_tune (void)
+static enum aarch64_parse_opt_result
+aarch64_parse_tune (const char *to_parse, const struct processor **res)
 {
   const struct processor *cpu;
-  char *str = (char *) alloca (strlen (aarch64_tune_string) + 1);
-  strcpy (str, aarch64_tune_string);
+  char *str = xstrdup (to_parse);
+  strcpy (str, to_parse);
 
   /* Loop through the list of supported CPUs to find a match.  */
   for (cpu = all_cores; cpu->name != NULL; cpu++)
     {
       if (strcmp (cpu->name, str) == 0)
 	{
-	  selected_tune = cpu;
-	  return;
+	  *res = cpu;
+	  free (str);
+	  return AARCH64_PARSE_OK;
 	}
     }
 
   /* CPU name not found in list.  */
-  error ("unknown value %qs for -mtune", str);
-  return;
+  free (str);
+  return AARCH64_PARSE_INVALID_ARG;
 }
 
 /* Parse TOKEN, which has length LENGTH to see if it is an option
@@ -7473,85 +7507,253 @@ aarch64_parse_override_string (const char* input_string,
   free (string_root);
 }
 
-/* Implement TARGET_OPTION_OVERRIDE.  */
 
 static void
-aarch64_override_options (void)
+aarch64_override_options_after_change_1 (struct gcc_options *opts)
 {
-  /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
-     If either of -march or -mtune is given, they override their
-     respective component of -mcpu.
+  if (opts->x_flag_omit_frame_pointer)
+    opts->x_flag_omit_leaf_frame_pointer = false;
+  else if (opts->x_flag_omit_leaf_frame_pointer)
+    opts->x_flag_omit_frame_pointer = true;
 
-     So, first parse AARCH64_CPU_STRING, then the others, be careful
-     with -march as, if -mcpu is not present on the command line, march
-     must set a sensible default CPU.  */
-  if (aarch64_cpu_string)
+  /* If not opzimizing for size, set the default
+     alignment to what the target wants.  */
+  if (!opts->x_optimize_size)
     {
-      aarch64_parse_cpu ();
+      if (opts->x_align_loops <= 0)
+	opts->x_align_loops = aarch64_tune_params.loop_align;
+      if (opts->x_align_jumps <= 0)
+	opts->x_align_jumps = aarch64_tune_params.jump_align;
+      if (opts->x_align_functions <= 0)
+	opts->x_align_functions = aarch64_tune_params.function_align;
     }
+}
 
-  if (aarch64_arch_string)
+/* 'Unpack' up the internal tuning structs and update the options
+    in OPTS.  The caller must have set up selected_tune and selected_arch
+    as all the other target-specific codegen decisions are
+    derived from them.  */
+
+static void
+aarch64_override_options_internal (struct gcc_options *opts)
+{
+  aarch64_tune_flags = selected_tune->flags;
+  aarch64_tune = selected_tune->sched_core;
+  /* Make a copy of the tuning parameters attached to the core, which
+     we may later overwrite.  */
+  aarch64_tune_params = *(selected_tune->tune);
+  aarch64_architecture_version = selected_arch->architecture_version;
+
+  if (opts->x_aarch64_override_tune_string)
+    aarch64_parse_override_string (opts->x_aarch64_override_tune_string,
+				  &aarch64_tune_params);
+
+  /* This target defaults to strict volatile bitfields.  */
+  if (opts->x_flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
+    opts->x_flag_strict_volatile_bitfields = 1;
+
+  if (opts->x_aarch64_fix_a53_err835769 == 2)
     {
-      aarch64_parse_arch ();
+#ifdef TARGET_FIX_ERR_A53_835769_DEFAULT
+      opts->x_aarch64_fix_a53_err835769 = 1;
+#else
+      opts->x_aarch64_fix_a53_err835769 = 0;
+#endif
     }
 
-  if (aarch64_tune_string)
+  /* -mgeneral-regs-only sets a mask in target_flags, make sure that
+     aarch64_isa_flags does not contain the FP/SIMD/Crypto feature flags
+     in case some code tries reading aarch64_isa_flags directly to check if
+     FP is available.  Reuse the aarch64_parse_extension machinery since it
+     knows how to disable any other flags that fp implies.  */
+  if (TARGET_GENERAL_REGS_ONLY_P (opts->x_target_flags))
     {
-      aarch64_parse_tune ();
+      /* aarch64_parse_extension takes char* rather than const char* because
+	 it is usually called from within other parsing functions.  */
+      char tmp_str[6];
+      strcpy (tmp_str, "+nofp");
+      aarch64_parse_extension (tmp_str, &aarch64_isa_flags);
     }
 
-#ifndef HAVE_AS_MABI_OPTION
-  /* The compiler may have been configured with 2.23.* binutils, which does
-     not have support for ILP32.  */
-  if (TARGET_ILP32)
-    error ("Assembler does not support -mabi=ilp32");
-#endif
+  initialize_aarch64_code_model (opts);
 
-  initialize_aarch64_code_model ();
+  aarch64_override_options_after_change_1 (opts);
+}
 
-  aarch64_build_bitmask_table ();
+/* Validate a command-line -mcpu option.  Parse the cpu and extensions (if any)
+   specified in STR and throw errors if appropriate.  Put the results if
+   they are valid in RES and ISA_FLAGS.  */
+static void
+aarch64_validate_mcpu (const char *str, const struct processor **res,
+		       unsigned long *isa_flags)
+{
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_cpu (str, res, isa_flags);
 
-  /* This target defaults to strict volatile bitfields.  */
-  if (flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
-    flag_strict_volatile_bitfields = 1;
+  if (parse_res == AARCH64_PARSE_OK)
+    return;
+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing cpu name in -mcpu=%qs", str);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for -mcpu", str);
+	break;
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier in -mcpu=%qs", str);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+}
+
+/* Validate a command-line -march option.  Parse the arch and extensions
+   (if any) specified in STR and throw errors if appropriate.  Put the
+   results, if they are valid, in RES and ISA_FLAGS.  */
+static void
+aarch64_validate_march (const char *str, const struct processor **res,
+		       unsigned long *isa_flags)
+{
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_arch (str, res, isa_flags);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    return;
+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing arch name in -march=%qs", str);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for -march", str);
+	break;
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier in -march=%qs", str);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+}
+
+/* Validate a command-line -mtune option.  Parse the cpu
+   specified in STR and throw errors if appropriate.  Put the
+   result, if it is valid, in RES.  */
+static void
+aarch64_validate_mtune (const char *str, const struct processor **res)
+{
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_tune (str, res);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    return;
+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing cpu name in -mtune=%qs", str);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for -mtune", str);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+}
+
+/* Implement TARGET_OPTION_OVERRIDE.  This is called once in the beginning
+   and is used to parse the -m{cpu,tune,arch} strings and setup the initial
+   tuning structs.  In particular it must set selected_tune and
+   aarch64_isa_flags that define the available ISA features and tuning
+   decisions.  It must also set selected_arch as this will be used to
+   output the .arch asm tags for each function.  */
+
+static void
+aarch64_override_options (void)
+{
+  unsigned long cpu_isa = 0;
+  unsigned long arch_isa = 0;
+  aarch64_isa_flags = 0;
+
+  selected_cpu = NULL;
+  selected_arch = NULL;
+  selected_tune = NULL;
+
+  /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
+     If either of -march or -mtune is given, they override their
+     respective component of -mcpu.  */
+  if (aarch64_cpu_string)
+    aarch64_validate_mcpu (aarch64_cpu_string, &selected_cpu, &cpu_isa);
+
+  if (aarch64_arch_string)
+    aarch64_validate_march (aarch64_arch_string, &selected_arch, &arch_isa);
+
+  if (aarch64_tune_string)
+    aarch64_validate_mtune (aarch64_tune_string, &selected_tune);
 
   /* If the user did not specify a processor, choose the default
      one for them.  This will be the CPU set during configuration using
      --with-cpu, otherwise it is "generic".  */
   if (!selected_cpu)
     {
-      selected_cpu = &all_cores[TARGET_CPU_DEFAULT & 0x3f];
-      aarch64_isa_flags = TARGET_CPU_DEFAULT >> 6;
+      if (selected_arch)
+	{
+	  selected_cpu = &all_cores[selected_arch->ident];
+	  aarch64_isa_flags = arch_isa;
+	}
+      else
+	{
+	  selected_cpu = &all_cores[TARGET_CPU_DEFAULT & 0x3f];
+	  aarch64_isa_flags = TARGET_CPU_DEFAULT >> 6;
+	}
     }
+  /* If both -mcpu and -march are specified check that they are architecturally
+     compatible, warn if they're not and prefer the -march ISA flags.  */
+  else if (selected_arch)
+    {
+      if (selected_arch->arch != selected_cpu->arch)
+	{
+	  warning (0, "switch -mcpu=%s conflicts with -march=%s switch",
+		       all_architectures[selected_cpu->arch].name,
+		       selected_arch->name);
+	}
+      aarch64_isa_flags = arch_isa;
+    }
+  /* -mcpu but no -march.  */
+  else
+    aarch64_isa_flags = cpu_isa;
 
-  gcc_assert (selected_cpu);
+  /* Set the arch as well as we will need it when outputing
+     the .arch directive in assembly.  */
+  if (!selected_arch)
+    {
+      gcc_assert (selected_cpu);
+      selected_arch = &all_architectures[selected_cpu->arch];
+    }
 
   if (!selected_tune)
     selected_tune = selected_cpu;
 
-  aarch64_tune_flags = selected_tune->flags;
-  aarch64_tune = selected_tune->sched_core;
-  /* Make a copy of the tuning parameters attached to the core, which
-     we may later overwrite.  */
-  aarch64_tune_params = *(selected_tune->tune);
-  aarch64_architecture_version = selected_cpu->architecture_version;
+#ifndef HAVE_AS_MABI_OPTION
+  /* The compiler may have been configured with 2.23.* binutils, which does
+     not have support for ILP32.  */
+  if (TARGET_ILP32)
+    error ("Assembler does not support -mabi=ilp32");
+#endif
 
-  if (aarch64_override_tune_string)
-    aarch64_parse_override_string (aarch64_override_tune_string,
-				   &aarch64_tune_params);
+  aarch64_build_bitmask_table ();
 
-  if (aarch64_fix_a53_err835769 == 2)
-    {
-#ifdef TARGET_FIX_ERR_A53_835769_DEFAULT
-      aarch64_fix_a53_err835769 = 1;
-#else
-      aarch64_fix_a53_err835769 = 0;
-#endif
-    }
+  aarch64_override_options_internal (&global_options);
+
+  /* Save these options as the default ones in case we push and pop them later
+     while processing functions with potential target attributes.  */
+  target_option_default_node = target_option_current_node
+      = build_target_option_node (&global_options);
 
   aarch64_register_fma_steering ();
 
-  aarch64_override_options_after_change ();
 }
 
 /* Implement targetm.override_options_after_change.  */
@@ -7575,6 +7777,8 @@ aarch64_override_options_after_change (void)
       if (align_functions <= 0)
 	align_functions = aarch64_tune_params.function_align;
     }
+
+  aarch64_override_options_after_change_1 (&global_options);
 }
 
 static struct machine_function *
@@ -7593,11 +7797,11 @@ aarch64_init_expanders (void)
 
 /* A checking mechanism for the implementation of the various code models.  */
 static void
-initialize_aarch64_code_model (void)
+initialize_aarch64_code_model (struct gcc_options *opts)
 {
-   if (flag_pic)
+   if (opts->x_flag_pic)
      {
-       switch (aarch64_cmodel_var)
+       switch (opts->x_aarch64_cmodel_var)
 	 {
 	 case AARCH64_CMODEL_TINY:
 	   aarch64_cmodel = AARCH64_CMODEL_TINY_PIC;
@@ -7613,13 +7817,13 @@ initialize_aarch64_code_model (void)
 	   break;
 	 case AARCH64_CMODEL_LARGE:
 	   sorry ("code model %qs with -f%s", "large",
-		  flag_pic > 1 ? "PIC" : "pic");
+		  opts->x_flag_pic > 1 ? "PIC" : "pic");
 	 default:
 	   gcc_unreachable ();
 	 }
      }
    else
-     aarch64_cmodel = aarch64_cmodel_var;
+     aarch64_cmodel = opts->x_aarch64_cmodel_var;
 }
 
 /* Return true if SYMBOL_REF X binds locally.  */
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 98ef9f6..c9c0aff 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -48,17 +48,6 @@ Enum(cmodel) String(small) Value(AARCH64_CMODEL_SMALL)
 EnumValue
 Enum(cmodel) String(large) Value(AARCH64_CMODEL_LARGE)
 
-; The cpu/arch option names to use in cpu/arch selection.
-
-Variable
-const char *aarch64_arch_string
-
-Variable
-const char *aarch64_cpu_string
-
-Variable
-const char *aarch64_tune_string
-
 mbig-endian
 Target Report RejectNegative Mask(BIG_END)
 Assume target CPU is configured as big endian
diff --git a/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c b/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c
index 155def0..807e325 100644
--- a/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c
@@ -1,4 +1,4 @@
-/* { dg-error "unknown" "" {target "aarch64*-*-*" } } */
+/* { dg-error "invalid feature" "" {target "aarch64*-*-*" } } */
 /* { dg-options "-O2 -mcpu=cortex-a53+dummy" } */
 
 void f ()

Re: [PATCH][AArch64][3/14] Refactor option override code

[James Greenhalgh]

On Thu, Jul 16, 2015 at 04:20:37PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> This one is more meaty than the previous ones. It buffs up the parsing functions for
> the mcpu, march, mtune options, decouples them and makes them return an enum describing
> the errors that may occur.  This will allow us to use these functions in other contexts
> beyond aarch64_override_options.
> 
> aarch64_override_options itself gets an overhaul and is split up into code that must run
> only once after the command line option have been processed, and code that has to be run
> every time the backend-specific state changes (after SWITCHABLE_TARGET is implemented).
> 
> The stuff that must be run every time the backend state changes is put into
> aarch64_override_options_internal.
> 
> Also, this patch deletes the declarations of aarch64_{arch,cpu,tune}_string from aarch64.opt
> as they are superfluous since the march, mtune and mcpu option specification implicitly
> declares these variables.
> 
> This patch looks large, but a lot of it is moving code around...
> 
> Bootstrapped and tested as part of the series on aarch64.
> 
> Ok for trunk?

I'm a bit hazy on the logic of one part, that is the refactoring of
aarch64_override_options_after_change in to
aarch64_override_options_after_change_1.

It seems like if we have this hunk:

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 32b974a..5ea65e3 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -7473,85 +7507,253 @@ aarch64_parse_override_string (const char* input_string,
>    free (string_root);
>  }
>  
> -/* Implement TARGET_OPTION_OVERRIDE.  */
>  
>  static void
> -aarch64_override_options (void)
> +aarch64_override_options_after_change_1 (struct gcc_options *opts)
>  {
> -  /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
> -     If either of -march or -mtune is given, they override their
> -     respective component of -mcpu.
> +  if (opts->x_flag_omit_frame_pointer)
> +    opts->x_flag_omit_leaf_frame_pointer = false;
> +  else if (opts->x_flag_omit_leaf_frame_pointer)
> +    opts->x_flag_omit_frame_pointer = true;
>  
> -     So, first parse AARCH64_CPU_STRING, then the others, be careful
> -     with -march as, if -mcpu is not present on the command line, march
> -     must set a sensible default CPU.  */
> -  if (aarch64_cpu_string)
> +  /* If not opzimizing for size, set the default
> +     alignment to what the target wants.  */
> +  if (!opts->x_optimize_size)
>      {
> -      aarch64_parse_cpu ();
> +      if (opts->x_align_loops <= 0)
> +	opts->x_align_loops = aarch64_tune_params.loop_align;
> +      if (opts->x_align_jumps <= 0)
> +	opts->x_align_jumps = aarch64_tune_params.jump_align;
> +      if (opts->x_align_functions <= 0)
> +	opts->x_align_functions = aarch64_tune_params.function_align;
>      }
> +}

Then this code left behind in aarch64_override_options_after_change :
>
>  if (flag_omit_frame_pointer)
>    flag_omit_leaf_frame_pointer = false;
>  else if (flag_omit_leaf_frame_pointer)
>    flag_omit_frame_pointer = true;
>
>  /* If not optimizing for size, set the default
>     alignment to what the target wants */
>  if (!optimize_size)
>    {
>      if (align_loops <= 0)
>	align_loops = aarch64_tune_params.loop_align;
>      if (align_jumps <= 0)
>	align_jumps = aarch64_tune_params.jump_align;
>      if (align_functions <= 0)
>	align_functions = aarch64_tune_params.function_align;
>    }

is redundant/misleading.

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 32b974a..5ea65e3 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -7101,12 +7101,27 @@ aarch64_add_stmt_cost (void *data, int count, enum vect_cost_for_stmt kind,
>    return retval;
>  }
>  
> -static void initialize_aarch64_code_model (void);
> +static void initialize_aarch64_code_model (struct gcc_options *);
>  
> -/* Parse the architecture extension string.  */
> +/* Enum describing the various ways that the
> +   aarch64_parse_{arch,tune,cpu,extension} functions can fail.
> +   This way their callers can choose what kind of error to give.  */
>  
> -static void
> -aarch64_parse_extension (char *str)
> +enum aarch64_parse_opt_result
> +{
> +  AARCH64_PARSE_OK,			/* Parsing was successful.  */
> +  AARCH64_PARSE_MISSING_ARG,		/* Missing argument.  */
> +  AARCH64_PARSE_INVALID_FEATURE,	/* Invalid feature modifier.  */
> +  AARCH64_PARSE_INVALID_ARG		/* Invalid arch, tune, cpu arg.  */
> +};
> +
> +

Extra newline here.

> -/* Parse the ARCH string.  */
> +/* Parse the TO_PARSE string and put the architecture struct that it
> +   selects into RES and the architectural features into ISA_FLAGS.
> +   Return an aarch64_parse_opt_result describing the parse result.
> +   If there is an error parsing, RES and ISA_FLAGS are left unchanged.  */
>  
> -static void
> -aarch64_parse_arch (void)
> +static enum aarch64_parse_opt_result
> +aarch64_parse_arch (const char *to_parse, const struct processor **res,
> +		    unsigned long *isa_flags)
>  {
>    char *ext;
>    const struct processor *arch;
> -  char *str = (char *) alloca (strlen (aarch64_arch_string) + 1);
> +  char *str = xstrdup (to_parse);
>    size_t len;
>  
> -  strcpy (str, aarch64_arch_string);
> +  strcpy (str, to_parse);

Is this really needed after the xstrdup you used above?

> -/* Parse the CPU string.  */
> +/* Parse the TO_PARSE string and put the result tuning in RES and the
> +   architecture flags in ISA_FLAGS.  Return an aarch64_parse_opt_result
> +   describing the parse result.  If there is an error parsing, RES and
> +   ISA_FLAGS are left unchanged.  */
>  
> -static void
> -aarch64_parse_cpu (void)
> +static enum aarch64_parse_opt_result
> +aarch64_parse_cpu (const char *to_parse, const struct processor **res,
> +		   unsigned long *isa_flags)
>  {
>    char *ext;
>    const struct processor *cpu;
> -  char *str = (char *) alloca (strlen (aarch64_cpu_string) + 1);
> +  char *str = xstrdup (to_parse);
>    size_t len;
>  
> -  strcpy (str, aarch64_cpu_string);
> +  strcpy (str, to_parse);

As above, is this needed after the xstrdup?

> -/* Parse the TUNE string.  */
> +/* Parse the TO_PARSE string and put the cpu it selects into RES.
> +   Return an aarch64_parse_opt_result describing the parse result.
> +   If the parsing fails the RES does not change.  */
>  
> -static void
> -aarch64_parse_tune (void)
> +static enum aarch64_parse_opt_result
> +aarch64_parse_tune (const char *to_parse, const struct processor **res)
>  {
>    const struct processor *cpu;
> -  char *str = (char *) alloca (strlen (aarch64_tune_string) + 1);
> -  strcpy (str, aarch64_tune_string);
> +  char *str = xstrdup (to_parse);
> +  strcpy (str, to_parse);

Likewise.

> +/* 'Unpack' up the internal tuning structs and update the options
> +    in OPTS.  The caller must have set up selected_tune and selected_arch
> +    as all the other target-specific codegen decisions are
> +    derived from them.  */
> +
> +static void
> +aarch64_override_options_internal (struct gcc_options *opts)
> +{
> +  aarch64_tune_flags = selected_tune->flags;
> +  aarch64_tune = selected_tune->sched_core;
> +  /* Make a copy of the tuning parameters attached to the core, which
> +     we may later overwrite.  */
> +  aarch64_tune_params = *(selected_tune->tune);
> +  aarch64_architecture_version = selected_arch->architecture_version;
> +
> +  if (opts->x_aarch64_override_tune_string)
> +    aarch64_parse_override_string (opts->x_aarch64_override_tune_string,
> +				  &aarch64_tune_params);
> +
> +  /* This target defaults to strict volatile bitfields.  */
> +  if (opts->x_flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
> +    opts->x_flag_strict_volatile_bitfields = 1;
> +
> +  if (opts->x_aarch64_fix_a53_err835769 == 2)
>      {
> -      aarch64_parse_arch ();
> +#ifdef TARGET_FIX_ERR_A53_835769_DEFAULT
> +      opts->x_aarch64_fix_a53_err835769 = 1;
> +#else
> +      opts->x_aarch64_fix_a53_err835769 = 0;
> +#endif
>      }
>  
> -  if (aarch64_tune_string)
> +  /* -mgeneral-regs-only sets a mask in target_flags, make sure that
> +     aarch64_isa_flags does not contain the FP/SIMD/Crypto feature flags
> +     in case some code tries reading aarch64_isa_flags directly to check if
> +     FP is available.  Reuse the aarch64_parse_extension machinery since it
> +     knows how to disable any other flags that fp implies.  */
> +  if (TARGET_GENERAL_REGS_ONLY_P (opts->x_target_flags))
>      {
> -      aarch64_parse_tune ();
> +      /* aarch64_parse_extension takes char* rather than const char* because
> +	 it is usually called from within other parsing functions.  */
> +      char tmp_str[6];
> +      strcpy (tmp_str, "+nofp");

Just:

  char tmp_str[] = "+nofp";

?

Thanks,
James

Re: [PATCH][AArch64][3/14] Refactor option override code

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 10945 bytes --]


On 20/07/15 17:44, James Greenhalgh wrote:
> On Thu, Jul 16, 2015 at 04:20:37PM +0100, Kyrill Tkachov wrote:
>> Hi all,
>>
>> This one is more meaty than the previous ones. It buffs up the parsing functions for
>> the mcpu, march, mtune options, decouples them and makes them return an enum describing
>> the errors that may occur.  This will allow us to use these functions in other contexts
>> beyond aarch64_override_options.
>>
>> aarch64_override_options itself gets an overhaul and is split up into code that must run
>> only once after the command line option have been processed, and code that has to be run
>> every time the backend-specific state changes (after SWITCHABLE_TARGET is implemented).
>>
>> The stuff that must be run every time the backend state changes is put into
>> aarch64_override_options_internal.
>>
>> Also, this patch deletes the declarations of aarch64_{arch,cpu,tune}_string from aarch64.opt
>> as they are superfluous since the march, mtune and mcpu option specification implicitly
>> declares these variables.
>>
>> This patch looks large, but a lot of it is moving code around...
>>
>> Bootstrapped and tested as part of the series on aarch64.
>>
>> Ok for trunk?
> I'm a bit hazy on the logic of one part, that is the refactoring of
> aarch64_override_options_after_change in to
> aarch64_override_options_after_change_1.
>
> It seems like if we have this hunk:
>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 32b974a..5ea65e3 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -7473,85 +7507,253 @@ aarch64_parse_override_string (const char* input_string,
>>     free (string_root);
>>   }
>>   
>> -/* Implement TARGET_OPTION_OVERRIDE.  */
>>   
>>   static void
>> -aarch64_override_options (void)
>> +aarch64_override_options_after_change_1 (struct gcc_options *opts)
>>   {
>> -  /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
>> -     If either of -march or -mtune is given, they override their
>> -     respective component of -mcpu.
>> +  if (opts->x_flag_omit_frame_pointer)
>> +    opts->x_flag_omit_leaf_frame_pointer = false;
>> +  else if (opts->x_flag_omit_leaf_frame_pointer)
>> +    opts->x_flag_omit_frame_pointer = true;
>>   
>> -     So, first parse AARCH64_CPU_STRING, then the others, be careful
>> -     with -march as, if -mcpu is not present on the command line, march
>> -     must set a sensible default CPU.  */
>> -  if (aarch64_cpu_string)
>> +  /* If not opzimizing for size, set the default
>> +     alignment to what the target wants.  */
>> +  if (!opts->x_optimize_size)
>>       {
>> -      aarch64_parse_cpu ();
>> +      if (opts->x_align_loops <= 0)
>> +	opts->x_align_loops = aarch64_tune_params.loop_align;
>> +      if (opts->x_align_jumps <= 0)
>> +	opts->x_align_jumps = aarch64_tune_params.jump_align;
>> +      if (opts->x_align_functions <= 0)
>> +	opts->x_align_functions = aarch64_tune_params.function_align;
>>       }
>> +}
> Then this code left behind in aarch64_override_options_after_change :
>>   if (flag_omit_frame_pointer)
>>     flag_omit_leaf_frame_pointer = false;
>>   else if (flag_omit_leaf_frame_pointer)
>>     flag_omit_frame_pointer = true;
>>
>>   /* If not optimizing for size, set the default
>>      alignment to what the target wants */
>>   if (!optimize_size)
>>     {
>>       if (align_loops <= 0)
>> 	align_loops = aarch64_tune_params.loop_align;
>>       if (align_jumps <= 0)
>> 	align_jumps = aarch64_tune_params.jump_align;
>>       if (align_functions <= 0)
>> 	align_functions = aarch64_tune_params.function_align;
>>     }
> is redundant/misleading.

Yes, this is redundant.

>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 32b974a..5ea65e3 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -7101,12 +7101,27 @@ aarch64_add_stmt_cost (void *data, int count, enum vect_cost_for_stmt kind,
>>     return retval;
>>   }
>>   
>> -static void initialize_aarch64_code_model (void);
>> +static void initialize_aarch64_code_model (struct gcc_options *);
>>   
>> -/* Parse the architecture extension string.  */
>> +/* Enum describing the various ways that the
>> +   aarch64_parse_{arch,tune,cpu,extension} functions can fail.
>> +   This way their callers can choose what kind of error to give.  */
>>   
>> -static void
>> -aarch64_parse_extension (char *str)
>> +enum aarch64_parse_opt_result
>> +{
>> +  AARCH64_PARSE_OK,			/* Parsing was successful.  */
>> +  AARCH64_PARSE_MISSING_ARG,		/* Missing argument.  */
>> +  AARCH64_PARSE_INVALID_FEATURE,	/* Invalid feature modifier.  */
>> +  AARCH64_PARSE_INVALID_ARG		/* Invalid arch, tune, cpu arg.  */
>> +};
>> +
>> +
> Extra newline here.
>
>> -/* Parse the ARCH string.  */
>> +/* Parse the TO_PARSE string and put the architecture struct that it
>> +   selects into RES and the architectural features into ISA_FLAGS.
>> +   Return an aarch64_parse_opt_result describing the parse result.
>> +   If there is an error parsing, RES and ISA_FLAGS are left unchanged.  */
>>   
>> -static void
>> -aarch64_parse_arch (void)
>> +static enum aarch64_parse_opt_result
>> +aarch64_parse_arch (const char *to_parse, const struct processor **res,
>> +		    unsigned long *isa_flags)
>>   {
>>     char *ext;
>>     const struct processor *arch;
>> -  char *str = (char *) alloca (strlen (aarch64_arch_string) + 1);
>> +  char *str = xstrdup (to_parse);
>>     size_t len;
>>   
>> -  strcpy (str, aarch64_arch_string);
>> +  strcpy (str, to_parse);
> Is this really needed after the xstrdup you used above?
>
>> -/* Parse the CPU string.  */
>> +/* Parse the TO_PARSE string and put the result tuning in RES and the
>> +   architecture flags in ISA_FLAGS.  Return an aarch64_parse_opt_result
>> +   describing the parse result.  If there is an error parsing, RES and
>> +   ISA_FLAGS are left unchanged.  */
>>   
>> -static void
>> -aarch64_parse_cpu (void)
>> +static enum aarch64_parse_opt_result
>> +aarch64_parse_cpu (const char *to_parse, const struct processor **res,
>> +		   unsigned long *isa_flags)
>>   {
>>     char *ext;
>>     const struct processor *cpu;
>> -  char *str = (char *) alloca (strlen (aarch64_cpu_string) + 1);
>> +  char *str = xstrdup (to_parse);
>>     size_t len;
>>   
>> -  strcpy (str, aarch64_cpu_string);
>> +  strcpy (str, to_parse);
> As above, is this needed after the xstrdup?
>
>> -/* Parse the TUNE string.  */
>> +/* Parse the TO_PARSE string and put the cpu it selects into RES.
>> +   Return an aarch64_parse_opt_result describing the parse result.
>> +   If the parsing fails the RES does not change.  */
>>   
>> -static void
>> -aarch64_parse_tune (void)
>> +static enum aarch64_parse_opt_result
>> +aarch64_parse_tune (const char *to_parse, const struct processor **res)
>>   {
>>     const struct processor *cpu;
>> -  char *str = (char *) alloca (strlen (aarch64_tune_string) + 1);
>> -  strcpy (str, aarch64_tune_string);
>> +  char *str = xstrdup (to_parse);
>> +  strcpy (str, to_parse);
> Likewise.
>
>> +/* 'Unpack' up the internal tuning structs and update the options
>> +    in OPTS.  The caller must have set up selected_tune and selected_arch
>> +    as all the other target-specific codegen decisions are
>> +    derived from them.  */
>> +
>> +static void
>> +aarch64_override_options_internal (struct gcc_options *opts)
>> +{
>> +  aarch64_tune_flags = selected_tune->flags;
>> +  aarch64_tune = selected_tune->sched_core;
>> +  /* Make a copy of the tuning parameters attached to the core, which
>> +     we may later overwrite.  */
>> +  aarch64_tune_params = *(selected_tune->tune);
>> +  aarch64_architecture_version = selected_arch->architecture_version;
>> +
>> +  if (opts->x_aarch64_override_tune_string)
>> +    aarch64_parse_override_string (opts->x_aarch64_override_tune_string,
>> +				  &aarch64_tune_params);
>> +
>> +  /* This target defaults to strict volatile bitfields.  */
>> +  if (opts->x_flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
>> +    opts->x_flag_strict_volatile_bitfields = 1;
>> +
>> +  if (opts->x_aarch64_fix_a53_err835769 == 2)
>>       {
>> -      aarch64_parse_arch ();
>> +#ifdef TARGET_FIX_ERR_A53_835769_DEFAULT
>> +      opts->x_aarch64_fix_a53_err835769 = 1;
>> +#else
>> +      opts->x_aarch64_fix_a53_err835769 = 0;
>> +#endif
>>       }
>>   
>> -  if (aarch64_tune_string)
>> +  /* -mgeneral-regs-only sets a mask in target_flags, make sure that
>> +     aarch64_isa_flags does not contain the FP/SIMD/Crypto feature flags
>> +     in case some code tries reading aarch64_isa_flags directly to check if
>> +     FP is available.  Reuse the aarch64_parse_extension machinery since it
>> +     knows how to disable any other flags that fp implies.  */
>> +  if (TARGET_GENERAL_REGS_ONLY_P (opts->x_target_flags))
>>       {
>> -      aarch64_parse_tune ();
>> +      /* aarch64_parse_extension takes char* rather than const char* because
>> +	 it is usually called from within other parsing functions.  */
>> +      char tmp_str[6];
>> +      strcpy (tmp_str, "+nofp");
> Just:
>
>    char tmp_str[] = "+nofp";
>
> ?

Thanks for the review, here's an updated version.
In the end, I chose to retain the use alloca (other patches in the series
are reworked to use it too).

How's this?

Thanks,
Kyrill

2015-07-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

      * config/aarch64/aarch64.opt (aarch64_arch_string): Delete.
      (aarch64_cpu_string): Likewise.
      (aarch64_tune_string): Likewise.
      * config/aarch64/aarch64.c (aarch64_parse_opt_result): New enum.
      (aarch64_parse_extension): Return aarch64_parse_opt_result.
      Add extra argument to put result into.
      (aarch64_parse_arch): Likewise.  Do not set selected_cpu.
      (aarch64_parse_cpu): Add arguments to put results into. Return
      aarch64_parse_opt_result.
      (aarch64_parse_tune): Likewise.
      (aarch64_override_options_after_change_1): New function.
      (aarch64_override_options_internal): New function.
      (aarch64_validate_mcpu): Likewise.
      (aarch64_validate_march): Likewise.
      (aarch64_validate_mtune): Likewise.
      (aarch64_override_options): Update to reflect above changes.
      Move some logic into aarch64_override_options_internal.
      Initialize target_option_default_node and target_option_current_node.
      (aarch64_override_options_after_change): Move logic into
      aarch64_override_options_after_change_1 and call it with global_options.
      (initialize_aarch64_code_model): Take a gcc_options pointer and use the
      flag values from that.

>
> Thanks,
> James
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-attrs-3.patch --]
[-- Type: text/x-patch; name=aarch64-attrs-3.patch, Size: 21644 bytes --]

commit 243a6567f976508332dcdf3623952a0f5d5bf123
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed May 6 16:27:00 2015 +0100

    [AArch64][3/N] Refactor option override code

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 32b974a..2ed8505 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7101,12 +7101,26 @@ aarch64_add_stmt_cost (void *data, int count, enum vect_cost_for_stmt kind,
   return retval;
 }
 
-static void initialize_aarch64_code_model (void);
+static void initialize_aarch64_code_model (struct gcc_options *);
 
-/* Parse the architecture extension string.  */
+/* Enum describing the various ways that the
+   aarch64_parse_{arch,tune,cpu,extension} functions can fail.
+   This way their callers can choose what kind of error to give.  */
 
-static void
-aarch64_parse_extension (char *str)
+enum aarch64_parse_opt_result
+{
+  AARCH64_PARSE_OK,			/* Parsing was successful.  */
+  AARCH64_PARSE_MISSING_ARG,		/* Missing argument.  */
+  AARCH64_PARSE_INVALID_FEATURE,	/* Invalid feature modifier.  */
+  AARCH64_PARSE_INVALID_ARG		/* Invalid arch, tune, cpu arg.  */
+};
+
+/* Parse the architecture extension string STR and update ISA_FLAGS
+   with the architecture features turned on or off.  Return a
+   aarch64_parse_opt_result describing the result.  */
+
+static enum aarch64_parse_opt_result
+aarch64_parse_extension (char *str, unsigned long *isa_flags)
 {
   /* The extension string is parsed left to right.  */
   const struct aarch64_option_extension *opt = NULL;
@@ -7137,11 +7151,8 @@ aarch64_parse_extension (char *str)
 	adding_ext = 1;
 
       if (len == 0)
-	{
-	  error ("missing feature modifier after %qs", adding_ext ? "+"
-	                                                          : "+no");
-	  return;
-	}
+	return AARCH64_PARSE_MISSING_ARG;
+
 
       /* Scan over the extensions table trying to find an exact match.  */
       for (opt = all_extensions; opt->name != NULL; opt++)
@@ -7150,9 +7161,9 @@ aarch64_parse_extension (char *str)
 	    {
 	      /* Add or remove the extension.  */
 	      if (adding_ext)
-		aarch64_isa_flags |= opt->flags_on;
+		*isa_flags |= opt->flags_on;
 	      else
-		aarch64_isa_flags &= ~(opt->flags_off);
+		*isa_flags &= ~(opt->flags_off);
 	      break;
 	    }
 	}
@@ -7160,27 +7171,30 @@ aarch64_parse_extension (char *str)
       if (opt->name == NULL)
 	{
 	  /* Extension not found in list.  */
-	  error ("unknown feature modifier %qs", str);
-	  return;
+	  return AARCH64_PARSE_INVALID_FEATURE;
 	}
 
       str = ext;
     };
 
-  return;
+  return AARCH64_PARSE_OK;
 }
 
-/* Parse the ARCH string.  */
+/* Parse the TO_PARSE string and put the architecture struct that it
+   selects into RES and the architectural features into ISA_FLAGS.
+   Return an aarch64_parse_opt_result describing the parse result.
+   If there is an error parsing, RES and ISA_FLAGS are left unchanged.  */
 
-static void
-aarch64_parse_arch (void)
+static enum aarch64_parse_opt_result
+aarch64_parse_arch (const char *to_parse, const struct processor **res,
+		    unsigned long *isa_flags)
 {
   char *ext;
   const struct processor *arch;
-  char *str = (char *) alloca (strlen (aarch64_arch_string) + 1);
+  char *str = (char *) alloca (strlen (to_parse) + 1);
   size_t len;
 
-  strcpy (str, aarch64_arch_string);
+  strcpy (str, to_parse);
 
   ext = strchr (str, '+');
 
@@ -7190,55 +7204,52 @@ aarch64_parse_arch (void)
     len = strlen (str);
 
   if (len == 0)
-    {
-      error ("missing arch name in -march=%qs", str);
-      return;
-    }
+    return AARCH64_PARSE_MISSING_ARG;
 
-  /* Loop through the list of supported ARCHs to find a match.  */
+
+  /* Loop through the list of supported ARCHes to find a match.  */
   for (arch = all_architectures; arch->name != NULL; arch++)
     {
       if (strlen (arch->name) == len && strncmp (arch->name, str, len) == 0)
 	{
-	  selected_arch = arch;
-	  aarch64_isa_flags = selected_arch->flags;
-
-	  if (!selected_cpu)
-	    selected_cpu = &all_cores[selected_arch->ident];
+	  unsigned long isa_temp = arch->flags;
 
 	  if (ext != NULL)
 	    {
-	      /* ARCH string contains at least one extension.  */
-	      aarch64_parse_extension (ext);
-	    }
+	      /* TO_PARSE string contains at least one extension.  */
+	      enum aarch64_parse_opt_result ext_res
+		= aarch64_parse_extension (ext, &isa_temp);
 
-	  if (selected_arch->arch != selected_cpu->arch)
-	    {
-	      warning (0, "switch -mcpu=%s conflicts with -march=%s switch",
-		       all_architectures[selected_cpu->arch].name,
-		       selected_arch->name);
+	      if (ext_res != AARCH64_PARSE_OK)
+		return ext_res;
 	    }
-
-	  return;
+	  /* Extension parsing was successful.  Confirm the result
+	     arch and ISA flags.  */
+	  *res = arch;
+	  *isa_flags = isa_temp;
+	  return AARCH64_PARSE_OK;
 	}
     }
 
   /* ARCH name not found in list.  */
-  error ("unknown value %qs for -march", str);
-  return;
+  return AARCH64_PARSE_INVALID_ARG;
 }
 
-/* Parse the CPU string.  */
+/* Parse the TO_PARSE string and put the result tuning in RES and the
+   architecture flags in ISA_FLAGS.  Return an aarch64_parse_opt_result
+   describing the parse result.  If there is an error parsing, RES and
+   ISA_FLAGS are left unchanged.  */
 
-static void
-aarch64_parse_cpu (void)
+static enum aarch64_parse_opt_result
+aarch64_parse_cpu (const char *to_parse, const struct processor **res,
+		   unsigned long *isa_flags)
 {
   char *ext;
   const struct processor *cpu;
-  char *str = (char *) alloca (strlen (aarch64_cpu_string) + 1);
+  char *str = (char *) alloca (strlen (to_parse) + 1);
   size_t len;
 
-  strcpy (str, aarch64_cpu_string);
+  strcpy (str, to_parse);
 
   ext = strchr (str, '+');
 
@@ -7248,56 +7259,62 @@ aarch64_parse_cpu (void)
     len = strlen (str);
 
   if (len == 0)
-    {
-      error ("missing cpu name in -mcpu=%qs", str);
-      return;
-    }
+    return AARCH64_PARSE_MISSING_ARG;
+
 
   /* Loop through the list of supported CPUs to find a match.  */
   for (cpu = all_cores; cpu->name != NULL; cpu++)
     {
       if (strlen (cpu->name) == len && strncmp (cpu->name, str, len) == 0)
 	{
-	  selected_cpu = cpu;
-	  aarch64_isa_flags = selected_cpu->flags;
+	  unsigned long isa_temp = cpu->flags;
+
 
 	  if (ext != NULL)
 	    {
-	      /* CPU string contains at least one extension.  */
-	      aarch64_parse_extension (ext);
-	    }
+	      /* TO_PARSE string contains at least one extension.  */
+	      enum aarch64_parse_opt_result ext_res
+		= aarch64_parse_extension (ext, &isa_temp);
 
-	  return;
+	      if (ext_res != AARCH64_PARSE_OK)
+		return ext_res;
+	    }
+	  /* Extension parsing was successfull.  Confirm the result
+	     cpu and ISA flags.  */
+	  *res = cpu;
+	  *isa_flags = isa_temp;
+	  return AARCH64_PARSE_OK;
 	}
     }
 
   /* CPU name not found in list.  */
-  error ("unknown value %qs for -mcpu", str);
-  return;
+  return AARCH64_PARSE_INVALID_ARG;
 }
 
-/* Parse the TUNE string.  */
+/* Parse the TO_PARSE string and put the cpu it selects into RES.
+   Return an aarch64_parse_opt_result describing the parse result.
+   If the parsing fails the RES does not change.  */
 
-static void
-aarch64_parse_tune (void)
+static enum aarch64_parse_opt_result
+aarch64_parse_tune (const char *to_parse, const struct processor **res)
 {
   const struct processor *cpu;
-  char *str = (char *) alloca (strlen (aarch64_tune_string) + 1);
-  strcpy (str, aarch64_tune_string);
+  char *str = (char *) alloca (strlen (to_parse) + 1);
+
+  strcpy (str, to_parse);
 
   /* Loop through the list of supported CPUs to find a match.  */
   for (cpu = all_cores; cpu->name != NULL; cpu++)
     {
       if (strcmp (cpu->name, str) == 0)
 	{
-	  selected_tune = cpu;
-	  return;
+	  *res = cpu;
+	  return AARCH64_PARSE_OK;
 	}
     }
 
   /* CPU name not found in list.  */
-  error ("unknown value %qs for -mtune", str);
-  return;
+  return AARCH64_PARSE_INVALID_ARG;
 }
 
 /* Parse TOKEN, which has length LENGTH to see if it is an option
@@ -7473,85 +7490,254 @@ aarch64_parse_override_string (const char* input_string,
   free (string_root);
 }
 
-/* Implement TARGET_OPTION_OVERRIDE.  */
 
 static void
-aarch64_override_options (void)
+aarch64_override_options_after_change_1 (struct gcc_options *opts)
 {
-  /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
-     If either of -march or -mtune is given, they override their
-     respective component of -mcpu.
+  if (opts->x_flag_omit_frame_pointer)
+    opts->x_flag_omit_leaf_frame_pointer = false;
+  else if (opts->x_flag_omit_leaf_frame_pointer)
+    opts->x_flag_omit_frame_pointer = true;
 
-     So, first parse AARCH64_CPU_STRING, then the others, be careful
-     with -march as, if -mcpu is not present on the command line, march
-     must set a sensible default CPU.  */
-  if (aarch64_cpu_string)
+  /* If not opzimizing for size, set the default
+     alignment to what the target wants.  */
+  if (!opts->x_optimize_size)
     {
-      aarch64_parse_cpu ();
+      if (opts->x_align_loops <= 0)
+	opts->x_align_loops = aarch64_tune_params.loop_align;
+      if (opts->x_align_jumps <= 0)
+	opts->x_align_jumps = aarch64_tune_params.jump_align;
+      if (opts->x_align_functions <= 0)
+	opts->x_align_functions = aarch64_tune_params.function_align;
     }
+}
 
-  if (aarch64_arch_string)
+/* 'Unpack' up the internal tuning structs and update the options
+    in OPTS.  The caller must have set up selected_tune and selected_arch
+    as all the other target-specific codegen decisions are
+    derived from them.  */
+
+static void
+aarch64_override_options_internal (struct gcc_options *opts)
+{
+  aarch64_tune_flags = selected_tune->flags;
+  aarch64_tune = selected_tune->sched_core;
+  /* Make a copy of the tuning parameters attached to the core, which
+     we may later overwrite.  */
+  aarch64_tune_params = *(selected_tune->tune);
+  aarch64_architecture_version = selected_arch->architecture_version;
+
+  if (opts->x_aarch64_override_tune_string)
+    aarch64_parse_override_string (opts->x_aarch64_override_tune_string,
+				  &aarch64_tune_params);
+
+  /* This target defaults to strict volatile bitfields.  */
+  if (opts->x_flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
+    opts->x_flag_strict_volatile_bitfields = 1;
+
+  if (opts->x_aarch64_fix_a53_err835769 == 2)
     {
-      aarch64_parse_arch ();
+#ifdef TARGET_FIX_ERR_A53_835769_DEFAULT
+      opts->x_aarch64_fix_a53_err835769 = 1;
+#else
+      opts->x_aarch64_fix_a53_err835769 = 0;
+#endif
     }
 
-  if (aarch64_tune_string)
+  /* -mgeneral-regs-only sets a mask in target_flags, make sure that
+     aarch64_isa_flags does not contain the FP/SIMD/Crypto feature flags
+     in case some code tries reading aarch64_isa_flags directly to check if
+     FP is available.  Reuse the aarch64_parse_extension machinery since it
+     knows how to disable any other flags that fp implies.  */
+  if (TARGET_GENERAL_REGS_ONLY_P (opts->x_target_flags))
     {
-      aarch64_parse_tune ();
+      /* aarch64_parse_extension takes char* rather than const char* because
+	 it is usually called from within other parsing functions.  */
+      char tmp_str[] = "+nofp";
+      aarch64_parse_extension (tmp_str, &aarch64_isa_flags);
     }
 
-#ifndef HAVE_AS_MABI_OPTION
-  /* The compiler may have been configured with 2.23.* binutils, which does
-     not have support for ILP32.  */
-  if (TARGET_ILP32)
-    error ("Assembler does not support -mabi=ilp32");
-#endif
+  initialize_aarch64_code_model (opts);
 
-  initialize_aarch64_code_model ();
+  aarch64_override_options_after_change_1 (opts);
+}
 
-  aarch64_build_bitmask_table ();
+/* Validate a command-line -mcpu option.  Parse the cpu and extensions (if any)
+   specified in STR and throw errors if appropriate.  Put the results if
+   they are valid in RES and ISA_FLAGS.  */
+static void
+aarch64_validate_mcpu (const char *str, const struct processor **res,
+		       unsigned long *isa_flags)
+{
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_cpu (str, res, isa_flags);
 
-  /* This target defaults to strict volatile bitfields.  */
-  if (flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
-    flag_strict_volatile_bitfields = 1;
+  if (parse_res == AARCH64_PARSE_OK)
+    return;
+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing cpu name in -mcpu=%qs", str);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for -mcpu", str);
+	break;
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier in -mcpu=%qs", str);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+}
+
+/* Validate a command-line -march option.  Parse the arch and extensions
+   (if any) specified in STR and throw errors if appropriate.  Put the
+   results, if they are valid, in RES and ISA_FLAGS.  */
+static void
+aarch64_validate_march (const char *str, const struct processor **res,
+		       unsigned long *isa_flags)
+{
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_arch (str, res, isa_flags);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    return;
+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing arch name in -march=%qs", str);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for -march", str);
+	break;
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier in -march=%qs", str);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+}
+
+/* Validate a command-line -mtune option.  Parse the cpu
+   specified in STR and throw errors if appropriate.  Put the
+   result, if it is valid, in RES.  */
+static void
+aarch64_validate_mtune (const char *str, const struct processor **res)
+{
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_tune (str, res);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    return;
+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing cpu name in -mtune=%qs", str);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for -mtune", str);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+}
+
+/* Implement TARGET_OPTION_OVERRIDE.  This is called once in the beginning
+   and is used to parse the -m{cpu,tune,arch} strings and setup the initial
+   tuning structs.  In particular it must set selected_tune and
+   aarch64_isa_flags that define the available ISA features and tuning
+   decisions.  It must also set selected_arch as this will be used to
+   output the .arch asm tags for each function.  */
+
+static void
+aarch64_override_options (void)
+{
+  unsigned long cpu_isa = 0;
+  unsigned long arch_isa = 0;
+  aarch64_isa_flags = 0;
+
+  selected_cpu = NULL;
+  selected_arch = NULL;
+  selected_tune = NULL;
+
+  /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
+     If either of -march or -mtune is given, they override their
+     respective component of -mcpu.  */
+  if (aarch64_cpu_string)
+    aarch64_validate_mcpu (aarch64_cpu_string, &selected_cpu, &cpu_isa);
+
+  if (aarch64_arch_string)
+    aarch64_validate_march (aarch64_arch_string, &selected_arch, &arch_isa);
+
+  if (aarch64_tune_string)
+    aarch64_validate_mtune (aarch64_tune_string, &selected_tune);
 
   /* If the user did not specify a processor, choose the default
      one for them.  This will be the CPU set during configuration using
      --with-cpu, otherwise it is "generic".  */
   if (!selected_cpu)
     {
-      selected_cpu = &all_cores[TARGET_CPU_DEFAULT & 0x3f];
-      aarch64_isa_flags = TARGET_CPU_DEFAULT >> 6;
+      if (selected_arch)
+	{
+	  selected_cpu = &all_cores[selected_arch->ident];
+	  aarch64_isa_flags = arch_isa;
+	}
+      else
+	{
+	  selected_cpu = &all_cores[TARGET_CPU_DEFAULT & 0x3f];
+	  aarch64_isa_flags = TARGET_CPU_DEFAULT >> 6;
+	}
+    }
+  /* If both -mcpu and -march are specified check that they are architecturally
+     compatible, warn if they're not and prefer the -march ISA flags.  */
+  else if (selected_arch)
+    {
+      if (selected_arch->arch != selected_cpu->arch)
+	{
+	  warning (0, "switch -mcpu=%s conflicts with -march=%s switch",
+		       all_architectures[selected_cpu->arch].name,
+		       selected_arch->name);
+	}
+      aarch64_isa_flags = arch_isa;
+    }
+  else
+    {
+      /* -mcpu but no -march.  */
+      aarch64_isa_flags = cpu_isa;
     }
 
-  gcc_assert (selected_cpu);
+  /* Set the arch as well as we will need it when outputing
+     the .arch directive in assembly.  */
+  if (!selected_arch)
+    {
+      gcc_assert (selected_cpu);
+      selected_arch = &all_architectures[selected_cpu->arch];
+    }
 
   if (!selected_tune)
     selected_tune = selected_cpu;
 
-  aarch64_tune_flags = selected_tune->flags;
-  aarch64_tune = selected_tune->sched_core;
-  /* Make a copy of the tuning parameters attached to the core, which
-     we may later overwrite.  */
-  aarch64_tune_params = *(selected_tune->tune);
-  aarch64_architecture_version = selected_cpu->architecture_version;
+#ifndef HAVE_AS_MABI_OPTION
+  /* The compiler may have been configured with 2.23.* binutils, which does
+     not have support for ILP32.  */
+  if (TARGET_ILP32)
+    error ("Assembler does not support -mabi=ilp32");
+#endif
 
-  if (aarch64_override_tune_string)
-    aarch64_parse_override_string (aarch64_override_tune_string,
-				   &aarch64_tune_params);
+  aarch64_build_bitmask_table ();
 
-  if (aarch64_fix_a53_err835769 == 2)
-    {
-#ifdef TARGET_FIX_ERR_A53_835769_DEFAULT
-      aarch64_fix_a53_err835769 = 1;
-#else
-      aarch64_fix_a53_err835769 = 0;
-#endif
-    }
+  aarch64_override_options_internal (&global_options);
+
+  /* Save these options as the default ones in case we push and pop them later
+     while processing functions with potential target attributes.  */
+  target_option_default_node = target_option_current_node
+      = build_target_option_node (&global_options);
 
   aarch64_register_fma_steering ();
 
-  aarch64_override_options_after_change ();
 }
 
 /* Implement targetm.override_options_after_change.  */
@@ -7559,22 +7745,7 @@ aarch64_override_options (void)
 static void
 aarch64_override_options_after_change (void)
 {
-  if (flag_omit_frame_pointer)
-    flag_omit_leaf_frame_pointer = false;
-  else if (flag_omit_leaf_frame_pointer)
-    flag_omit_frame_pointer = true;
-
-  /* If not optimizing for size, set the default
-     alignment to what the target wants */
-  if (!optimize_size)
-    {
-      if (align_loops <= 0)
-	align_loops = aarch64_tune_params.loop_align;
-      if (align_jumps <= 0)
-	align_jumps = aarch64_tune_params.jump_align;
-      if (align_functions <= 0)
-	align_functions = aarch64_tune_params.function_align;
-    }
+  aarch64_override_options_after_change_1 (&global_options);
 }
 
 static struct machine_function *
@@ -7593,11 +7764,11 @@ aarch64_init_expanders (void)
 
 /* A checking mechanism for the implementation of the various code models.  */
 static void
-initialize_aarch64_code_model (void)
+initialize_aarch64_code_model (struct gcc_options *opts)
 {
-   if (flag_pic)
+   if (opts->x_flag_pic)
      {
-       switch (aarch64_cmodel_var)
+       switch (opts->x_aarch64_cmodel_var)
 	 {
 	 case AARCH64_CMODEL_TINY:
 	   aarch64_cmodel = AARCH64_CMODEL_TINY_PIC;
@@ -7613,13 +7784,13 @@ initialize_aarch64_code_model (void)
 	   break;
 	 case AARCH64_CMODEL_LARGE:
 	   sorry ("code model %qs with -f%s", "large",
-		  flag_pic > 1 ? "PIC" : "pic");
+		  opts->x_flag_pic > 1 ? "PIC" : "pic");
 	 default:
 	   gcc_unreachable ();
 	 }
      }
    else
-     aarch64_cmodel = aarch64_cmodel_var;
+     aarch64_cmodel = opts->x_aarch64_cmodel_var;
 }
 
 /* Return true if SYMBOL_REF X binds locally.  */
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 98ef9f6..c9c0aff 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -48,17 +48,6 @@ Enum(cmodel) String(small) Value(AARCH64_CMODEL_SMALL)
 EnumValue
 Enum(cmodel) String(large) Value(AARCH64_CMODEL_LARGE)
 
-; The cpu/arch option names to use in cpu/arch selection.
-
-Variable
-const char *aarch64_arch_string
-
-Variable
-const char *aarch64_cpu_string
-
-Variable
-const char *aarch64_tune_string
-
 mbig-endian
 Target Report RejectNegative Mask(BIG_END)
 Assume target CPU is configured as big endian
diff --git a/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c b/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c
index 155def0..807e325 100644
--- a/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/cpu-diagnostics-3.c
@@ -1,4 +1,4 @@
-/* { dg-error "unknown" "" {target "aarch64*-*-*" } } */
+/* { dg-error "invalid feature" "" {target "aarch64*-*-*" } } */
 /* { dg-options "-O2 -mcpu=cortex-a53+dummy" } */
 
 void f ()

Re: [PATCH][AArch64][3/14] Refactor option override code

[James Greenhalgh]

On Fri, Jul 24, 2015 at 09:21:46AM +0100, Kyrill Tkachov wrote:
> 
> Thanks for the review, here's an updated version.
> In the end, I chose to retain the use alloca (other patches in the series
> are reworked to use it too).
> 
> How's this?

A nit or two from code you were moving or that got caught up in the diff,
but otherwise OK to commit with the issues highlighted below fixed up.

>  
> -  aarch64_build_bitmask_table ();
> +/* Validate a command-line -mcpu option.  Parse the cpu and extensions (if any)
> +   specified in STR and throw errors if appropriate.  Put the results if
> +   they are valid in RES and ISA_FLAGS.  */
> +static void
> +aarch64_validate_mcpu (const char *str, const struct processor **res,
> +		       unsigned long *isa_flags)

Newline between comment and function.

> +/* Validate a command-line -march option.  Parse the arch and extensions
> +   (if any) specified in STR and throw errors if appropriate.  Put the
> +   results, if they are valid, in RES and ISA_FLAGS.  */
> +static void
> +aarch64_validate_march (const char *str, const struct processor **res,
> +		       unsigned long *isa_flags)

And here.

> +/* Validate a command-line -mtune option.  Parse the cpu
> +   specified in STR and throw errors if appropriate.  Put the
> +   result, if it is valid, in RES.  */
> +static void
> +aarch64_validate_mtune (const char *str, const struct processor **res)

And here.

Thanks,
James