Re: [PATCH 1/2] s/390: Implement "target" attribute.

[Andreas Krebbel <krebbel@linux.vnet.ibm.com>]

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

Hi Dominik,

on-top of the discussions we had off-list I only have a few additional
comments/questions.

Apart from that the patch looks good to me. Thanks!

Bye,

-Andreas-

> diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
> index 43459c8..4cf0df7 100644
> --- a/gcc/common/config/s390/s390-common.c
> +++ b/gcc/common/config/s390/s390-common.c
> @@ -79,41 +79,27 @@ s390_option_init_struct (struct gcc_options *opts)
>
>  /* Implement TARGET_HANDLE_OPTION.  */
>
> -static bool
> -s390_handle_option (struct gcc_options *opts,
> +bool
> +s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>  		    struct gcc_options *opts_set ATTRIBUTE_UNUSED,
>    		    const struct cl_decoded_option *decoded,
>  		    location_t loc)
>  {
>    size_t code = decoded->opt_index;
> -  const char *arg = decoded->arg;
>    int value = decoded->value;
>
>    switch (code)
>      {
> -    case OPT_march_:
> -      opts->x_s390_arch_flags = processor_flags_table[value];
> -      opts->x_s390_arch_string = arg;
> -      return true;
> -
>      case OPT_mstack_guard_:
> -      if (exact_log2 (value) == -1)
> +      if (value != 0 && exact_log2 (value) == -1)
>  	error_at (loc, "stack guard value must be an exact power of 2");
>        return true;
>
>      case OPT_mstack_size_:
> -      if (exact_log2 (value) == -1)
> +      if (value != 0 && exact_log2 (value) == -1)
>  	error_at (loc, "stack size must be an exact power of 2");
>        return true;

This probably is supposed to allow disabling of stack_guard and
stack-size options with 0 settings. Would removing the
`RejectNegative' in s390.opt be an option? I'm not sure but perhaps we
discussed this off-list already.

...
> +/* Helper function that defines or undefines macros.  If SET is true, the macro
> +   MACRO_DEF is defined.  If SET is false, the macro MACRO_UNDEF is undefined.
> +   Nothing is done if SET and WAS_SET have the same value.  */
> +static void
> +s390_def_or_undef_macro (cpp_reader *pfile, bool set, bool was_set,
> +			 const char *macro_def, const char *macro_undef)
>  {
> -  cpp_assert (pfile, "cpu=s390");
> -  cpp_assert (pfile, "machine=s390");
> -  cpp_define (pfile, "__s390__");
> -  if (TARGET_ZARCH)
> -    cpp_define (pfile, "__zarch__");
> -  if (TARGET_64BIT)
> -    cpp_define (pfile, "__s390x__");
> -  if (TARGET_LONG_DOUBLE_128)
> -    cpp_define (pfile, "__LONG_DOUBLE_128__");
> -  if (TARGET_HTM)
> -    cpp_define (pfile, "__HTM__");
> -  if (TARGET_ZVECTOR)
> +  if (set == was_set)
> +    return;
> +  if (set)
> +    cpp_define (pfile, macro_def);
> +  else
> +    cpp_undef (pfile, macro_undef);
> +}
> +
> +/* Internal function to either define or undef the appropriate system
> +   macros.  */
> +static void
> +s390_cpu_cpp_builtins_internal (cpp_reader *pfile,
> +				struct cl_target_option *opts,
> +				struct cl_target_option *old_opts)
> +{
> +  bool old;
> +  bool set;
> +
> +  old = (!old_opts) ? false : TARGET_HTM_P (old_opts);
> +  set = TARGET_HTM_P (opts);
> +  s390_def_or_undef_macro (pfile, set, old, "__HTM__", "__HTM__");
> +
> +  old = (!old_opts) ? false : TARGET_ZVECTOR_P (old_opts->x_target_flags);
> +  set = TARGET_ZVECTOR_P (opts->x_target_flags);
> +  s390_def_or_undef_macro (pfile, set, old, "__VEC__=10301", "__VEC__");
> +  s390_def_or_undef_macro (pfile, set, old,
> +			   "__vector=__attribute__((vector_size(16)))",
> +			   "__vector__");
> +  s390_def_or_undef_macro (pfile, set, old,
> +			   "__bool=__attribute__((s390_vector_bool)) unsigned",
> +			   "__bool");
> +  if (!flag_iso)
>      {
> -      cpp_define (pfile, "__VEC__=10301");
> -      cpp_define (pfile, "__vector=__attribute__((vector_size(16)))");
> -      cpp_define (pfile, "__bool=__attribute__((s390_vector_bool)) unsigned");
> +      s390_def_or_undef_macro (pfile, set, old, "__VECTOR_KEYWORD_SUPPORTED__",
> +			       "__VECTOR_KEYWORD_SUPPORTED__");
> +      s390_def_or_undef_macro (pfile, set, old, "vector=vector", "vector");
> +      s390_def_or_undef_macro (pfile, set, old, "bool=bool", "bool");
>
> -      if (!flag_iso)
> +      if (set && __vector_keyword == NULL)
>  	{
> -	  cpp_define (pfile, "__VECTOR_KEYWORD_SUPPORTED__");
> -	  cpp_define (pfile, "vector=vector");
> -	  cpp_define (pfile, "bool=bool");
> -
>  	  __vector_keyword = get_identifier ("__vector");
>  	  C_CPP_HASHNODE (__vector_keyword)->flags |= NODE_CONDITIONAL;
>

Slightly better:

static void
s390_def_or_undef_macro (cpp_reader *pfile,
			 int mask,
			 struct cl_target_option *newopts,
			 struct cl_target_option *oldopts,
			 const char *macro_def, const char *macro_undef)
{
  bool old;
  bool set;

  old = (!old_opts) ? false : old_opts->x_target_flags & mask;
  set = new_opts->x_target_flags & mask;

  if (set == was_set)
    return;
  if (set)
    cpp_define (pfile, macro_def);
  else
    cpp_undef (pfile, macro_undef);
}

/* Internal function to either define or undef the appropriate system
   macros.  */
static void
s390_cpu_cpp_builtins_internal (cpp_reader *pfile,
				struct cl_target_option *opts,
				struct cl_target_option *old_opts)
{
  s390_def_or_undef_macro (pfile, MASK_OPT_HTM, opts, oldopts,
			   "__HTM__", "__HTM__");
  ...


> @@ -335,6 +355,87 @@ s390_cpu_cpp_builtins (cpp_reader *pfile)
>      }
>  }
>
> +/* Define platform dependent macros.  */
> +void
> +s390_cpu_cpp_builtins (cpp_reader *pfile)
> +{
> +  struct cl_target_option opts;
> +
> +  cpp_assert (pfile, "cpu=s390");
> +  cpp_assert (pfile, "machine=s390");
> +  cpp_define (pfile, "__s390__");
> +  if (TARGET_ZARCH)
> +    cpp_define (pfile, "__zarch__");
> +  if (TARGET_64BIT)
> +    cpp_define (pfile, "__s390x__");
> +  if (TARGET_LONG_DOUBLE_128)
> +    cpp_define (pfile, "__LONG_DOUBLE_128__");
> +  cl_target_option_save (&opts, &global_options);
> +  s390_cpu_cpp_builtins_internal (pfile, &opts, NULL);
> +}
> +
> +#if S390_USE_TARGET_ATTRIBUTE
> +/* Hook to validate the current #pragma GCC target and set the state, and
> +   update the macros based on what was changed.  If ARGS is NULL, then
> +   POP_TARGET is used to reset the options.  */
> +
> +static bool
> +s390_pragma_target_parse (tree args, tree pop_target)
> +{
> +  tree prev_tree = build_target_option_node (&global_options);
> +  tree cur_tree;
> +
> +  {
> +    gcc_options options;
> +    tree def_tree;
> +
> +    memcpy (&options, &global_options, sizeof (options));
options = global_options;

> +    def_tree = (pop_target ? pop_target : target_option_default_node);
> +    cl_target_option_restore (&options, TREE_TARGET_OPTION (def_tree));
> +    if (! args)
> +      cur_tree = def_tree;
> +    else
> +      {
> +	gcc_options options_set;
> +
> +	memcpy (&options_set, &global_options_set, sizeof (options_set));
options_set = global_options_set;

> +	cur_tree = s390_valid_target_attribute_tree (args, &options,
> +						     &options_set, true);
> +	if (!cur_tree || cur_tree == error_mark_node)
> +	  return false;
Don't you need to do:
global_options_set = options_set;

> +      }
> +    memcpy (&global_options, &options, sizeof (options));
global_options = options;

All the backup and restore of the option structures is probably done
here to prevent half-way parsed options from staying in the data
structures while running into an error - right?!  Is this really a
problem, e.g. the i386 back-end does not seem to do it?

...
> @@ -761,10 +784,30 @@ s390_expand_builtin (tree exp, rtx target, rtx subtarget ATTRIBUTE_UNUSED,
>    if (TARGET_DEBUG_ARG)
>      {
>        fprintf (stderr,
> -	       "s390_expand_builtin, code = %4d, %s\n",
> -	       (int)fcode, IDENTIFIER_POINTER (DECL_NAME (fndecl)));
> +	       "s390_expand_builtin, code = %4d, %s, bflags = 0x%x\n",
> +	       (int)fcode, IDENTIFIER_POINTER (DECL_NAME (fndecl)),
> +	       bflags_for_builtin (fcode));
>      }
>
> +  if (S390_USE_TARGET_ATTRIBUTE)
> +    {
> +      unsigned int bflags;
> +
> +      bflags = bflags_for_builtin (fcode);
> +      /* Cast to int to avoid Warning "comparison is always true".  */

What cast?
> +      if ((bflags & B_HTM) &&!TARGET_HTM)
> +     {
> +       error ("Builtin %qF is not supported without -mhtm "
...
> +static void
> +s390_option_override_internal (struct gcc_options *opts,
> +			       struct gcc_options *opts_set)
> +{
>    /* Architecture mode defaults according to ABI.  */
> -  if (!(target_flags_explicit & MASK_ZARCH))
> +  if (!(opts_set->x_target_flags & MASK_ZARCH))
>      {
>        if (TARGET_64BIT)
> -	target_flags |= MASK_ZARCH;
> +	opts->x_target_flags |= MASK_ZARCH;
>        else
> -	target_flags &= ~MASK_ZARCH;
> +	opts->x_target_flags &= ~MASK_ZARCH;
>      }
>
> -  /* Set the march default in case it hasn't been specified on
> -     cmdline.  */
> -  if (s390_arch == PROCESSOR_max)
> +  /* Set the march default in case it hasn't been specified on cmdline.  */
> +  if (opts->x_s390_arch == PROCESSOR_max)
>      {
> -      s390_arch_string = TARGET_ZARCH? "z900" : "g5";
> -      s390_arch = TARGET_ZARCH ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5;
> -      s390_arch_flags = processor_flags_table[(int)s390_arch];
> +      opts->x_s390_arch = TARGET_ZARCH_P (opts->x_target_flags)
> +	? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5;
> +      opts->x_s390_arch_specified = 0;
>      }
> -
> +  else
> +    OPTS->X_S390_ARCH_SPECIFIED = 1;
> +  OPTS->X_S390_ARCH_FLAGS = PROCESSOR_FLAGS_TABLE[(INT) OPTS->X_S390_ARCH];
>    /* DETERMINE PROCESSOR TO TUNE FOR.  */
> -  IF (S390_TUNE == PROCESSOR_MAX)
> +  IF (OPTS->X_S390_TUNE == PROCESSOR_MAX)
>      {
> -      S390_TUNE = S390_ARCH;
> -      S390_TUNE_FLAGS = S390_ARCH_FLAGS;
> +      OPTS->X_S390_TUNE = OPTS->X_S390_ARCH;
> +      OPTS->X_S390_TUNE_SPECIFIED = 0;
>      }
> +  ELSE
> +    OPTS->X_S390_TUNE_SPECIFIED = 1;
> +  OPTS->X_S390_TUNE_FLAGS = PROCESSOR_FLAGS_TABLE[OPTS->X_S390_TUNE];

Why do we need x_s390_arch_specified and x_s390_tune_specified?  You
should be able to use opts_set->x_s390_arch and opts_set->x_s390_tune
instead? (patch attached, your tests keep working with that change).

...
> +/* Return a TARGET_OPTION_NODE tree of the target options listed or NULL.  */
> +
> +tree
> +s390_valid_target_attribute_tree (tree args,
> +				  struct gcc_options *opts,
> +				  struct gcc_options *opts_set,
> +				  bool force_pragma)
> +{
> +  int orig_arch_specified = s390_arch_specified;
> +  int orig_tune_specified = s390_tune_specified;
> +  tree t = NULL_TREE;
> +  struct gcc_options new_opts_set;
> +
> +  memset (&new_opts_set, 0, sizeof (new_opts_set));
> +
> +  /* Process each of the options on the chain.  */
> +  if (! s390_valid_target_attribute_inner_p (args, opts, &new_opts_set,
> +					     force_pragma))
> +    return error_mark_node;
> +
> +  /* If some option was set (even if it has not changed), rerun
> +     s390_option_override_internal, and then save the options away.  */
> +  if (new_opts_set.x_target_flags
> +      || new_opts_set.x_s390_arch
> +      || new_opts_set.x_s390_tune
> +      || new_opts_set.x_s390_stack_guard
> +      || new_opts_set.x_s390_stack_size
> +      || new_opts_set.x_s390_branch_cost
> +      || new_opts_set.x_s390_warn_framesize
> +      || new_opts_set.x_s390_warn_dynamicstack_p
> +      )
> +    {
> +      /* If we are using the default tune= or arch=, undo the value assigned,
> +	 and use the default.  */
> +      if (!new_opts_set.x_s390_arch)
> +	{
> +	  if (!orig_arch_specified)
> +	    opts->x_s390_arch = PROCESSOR_max;
> +	}
> +      if (!new_opts_set.x_s390_tune && new_opts_set.x_s390_arch)
> +	{
> +	  if (opts->x_s390_arch != PROCESSOR_max)
> +	    opts->x_s390_tune = PROCESSOR_max;
> +	  else if (!orig_tune_specified)
> +	    opts->x_s390_tune = PROCESSOR_max;
> +	}
> +
> +      {
> +	struct gcc_options joined_opts_set;
> +	unsigned char *src = (unsigned char *)&new_opts_set;
> +	unsigned char *dest = (unsigned char *)&joined_opts_set;
> +	unsigned int i;
> +
> +	/* Make a joined copy of the original and the additional option flags.
> +	 */
> +	memcpy (&joined_opts_set, opts_set, sizeof (*opts_set));
joined_opts_set = opts_set;

> +	for (i = 0; i < sizeof(*opts_set); i++)
> +	  dest[i] |= src[i];
> +
> +	/* Do any overrides, such as arch=xxx, or tune=xxx support.  */
> +	s390_option_override_internal (opts, &joined_opts_set);
> +      }
> +
> +      /* Save the current options unless we are validating options for
> +	 #pragma.  */
> +      t = build_target_option_node (opts);
> +    }
> +
> +  return t;
> +}
> +
> +/* Hook to validate attribute((target("string"))).  */
> +
> +static bool
> +s390_valid_target_attribute_p (tree fndecl,
> +			       tree ARG_UNUSED (name),
> +			       tree args,
> +			       int ARG_UNUSED (flags))
> +{
> +  struct gcc_options func_options;
> +  tree new_target, new_optimize;
> +  bool ret = true;
> +
> +  /* attribute((target("default"))) does nothing, beyond
> +     affecting multi-versioning.  */
> +  if (TREE_VALUE (args)
> +      && TREE_CODE (TREE_VALUE (args)) == STRING_CST
> +      && TREE_CHAIN (args) == NULL_TREE
> +      && strcmp (TREE_STRING_POINTER (TREE_VALUE (args)), "default") == 0)
> +    return true;
> +
> +  tree old_optimize = build_optimization_node (&global_options);
> +
> +  /* Get the optimization options of the current function.  */
> +  tree func_optimize = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl);
> +
> +  if (!func_optimize)
> +    func_optimize = old_optimize;
> +
> +  /* Init func_options.  */
> +  memset (&func_options, 0, sizeof (func_options));
> +  init_options_struct (&func_options, NULL);
> +  lang_hooks.init_options_struct (&func_options);
> +
> +  cl_optimization_restore (&func_options, TREE_OPTIMIZATION (func_optimize));
> +
> +  /* Initialize func_options to the default before its target options can
> +     be set.  */
> +  cl_target_option_restore (&func_options,
> +			    TREE_TARGET_OPTION (target_option_default_node));
> +
> +  new_target = s390_valid_target_attribute_tree (args, &func_options,
> +						 &global_options_set,
> +						 (args ==
> +						  current_target_pragma));
> +  new_optimize = build_optimization_node (&func_options);
> +  if (new_target == error_mark_node)
> +    ret = false;
> +  else if (fndecl && new_target)
> +    {
> +      DECL_FUNCTION_SPECIFIC_TARGET (fndecl) = new_target;
> +      if (old_optimize != new_optimize)
> +	DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl) = new_optimize;
> +    }
> +
> +  return ret;
> +}
> +
> +/* Set targets globals to the default (or current #pragma GCC target
> +   if active).  Invalidate s390_previous_fndecl cache.  */
> +
> +void
> +s390_reset_previous_fndecl (void)
> +{
> +  tree new_tree = target_option_current_node;
> +  cl_target_option_restore (&global_options, TREE_TARGET_OPTION (new_tree));
> +  if (TREE_TARGET_GLOBALS (new_tree))
> +    restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
> +  else if (new_tree == target_option_default_node)
> +    restore_target_globals (&default_target_globals);
> +  else
> +    TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();
> +  s390_previous_fndecl = NULL_TREE;
> +}
> +
> +/* Establish appropriate back-end context for processing the function
> +   FNDECL.  The argument might be NULL to indicate processing at top
> +   level, outside of any function scope.  */
> +static void
> +s390_set_current_function (tree fndecl)
> +{
> +  /* Only change the context if the function changes.  This hook is called
> +     several times in the course of compiling a function, and we don't want to
> +     slow things down too much or call target_reinit when it isn't safe.  */
> +  if (fndecl == s390_previous_fndecl)
> +    return;
> +
> +  tree old_tree;
> +  if (s390_previous_fndecl == NULL_TREE)
> +    old_tree = target_option_current_node;
> +  else if (DECL_FUNCTION_SPECIFIC_TARGET (s390_previous_fndecl))
> +    old_tree = DECL_FUNCTION_SPECIFIC_TARGET (s390_previous_fndecl);
> +  else
> +    old_tree = target_option_default_node;
> +
> +  if (fndecl == NULL_TREE)
> +    {
> +      if (old_tree != target_option_current_node)
> +	s390_reset_previous_fndecl ();
> +      return;
> +    }
> +
> +  tree new_tree = DECL_FUNCTION_SPECIFIC_TARGET (fndecl);
> +  if (new_tree == NULL_TREE)
> +    new_tree = target_option_default_node;
> +
> +  if (old_tree != new_tree)
> +    {
> +      cl_target_option_restore (&global_options, TREE_TARGET_OPTION (new_tree));
> +      if (TREE_TARGET_GLOBALS (new_tree))
> +	restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
> +      else if (new_tree == target_option_default_node)
> +	restore_target_globals (&default_target_globals);
> +      else
> +	TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();

This last part is almost s390_reset_previous_fndecl. Perhaps it could
be merged?!

> +    }
> +  s390_previous_fndecl = fndecl;
> +}
> +#endif
...


[-- Attachment #2: targetattr-changes.patch --]
[-- Type: text/x-diff, Size: 5749 bytes --]

diff --git a/gcc/config/s390/s390-c.c b/gcc/config/s390/s390-c.c
index dc05c17..0c5fe57 100644
--- a/gcc/config/s390/s390-c.c
+++ b/gcc/config/s390/s390-c.c
@@ -389,7 +389,7 @@ s390_pragma_target_parse (tree args, tree pop_target)
     gcc_options options;
     tree def_tree;
 
-    memcpy (&options, &global_options, sizeof (options));
+    options = global_options;
     def_tree = (pop_target ? pop_target : target_option_default_node);
     cl_target_option_restore (&options, TREE_TARGET_OPTION (def_tree));
     if (! args)
@@ -398,13 +398,14 @@ s390_pragma_target_parse (tree args, tree pop_target)
       {
 	gcc_options options_set;
 
-	memcpy (&options_set, &global_options_set, sizeof (options_set));
+	options_set = global_options_set;
 	cur_tree = s390_valid_target_attribute_tree (args, &options,
 						     &options_set, true);
 	if (!cur_tree || cur_tree == error_mark_node)
 	  return false;
+	global_options_set = options_set;
       }
-    memcpy (&global_options, &options, sizeof (options));
+    global_options = options;
   }
 
   target_option_current_node = cur_tree;
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 3c5a042..a94420b 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -13532,23 +13532,15 @@ s390_option_override_internal (struct gcc_options *opts,
     }
 
   /* Set the march default in case it hasn't been specified on cmdline.  */
-  if (opts->x_s390_arch == PROCESSOR_max)
-    {
-      opts->x_s390_arch = TARGET_ZARCH_P (opts->x_target_flags)
-	? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5;
-      opts->x_s390_arch_specified = 0;
-    }
-  else
-    opts->x_s390_arch_specified = 1;
+  if (!opts_set->x_s390_arch)
+    opts->x_s390_arch = TARGET_ZARCH_P (opts->x_target_flags)
+      ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5;
+
   opts->x_s390_arch_flags = processor_flags_table[(int) opts->x_s390_arch];
   /* Determine processor to tune for.  */
-  if (opts->x_s390_tune == PROCESSOR_max)
-    {
-      opts->x_s390_tune = opts->x_s390_arch;
-      opts->x_s390_tune_specified = 0;
-    }
-  else
-    opts->x_s390_tune_specified = 1;
+  if (!opts_set->x_s390_tune)
+    opts->x_s390_tune = opts->x_s390_arch;
+
   opts->x_s390_tune_flags = processor_flags_table[opts->x_s390_tune];
 
   /* Sanity checks.  */
@@ -14033,8 +14025,6 @@ s390_valid_target_attribute_tree (tree args,
 				  struct gcc_options *opts_set,
 				  bool force_pragma)
 {
-  int orig_arch_specified = s390_arch_specified;
-  int orig_tune_specified = s390_tune_specified;
   tree t = NULL_TREE;
   struct gcc_options new_opts_set;
 
@@ -14054,45 +14044,29 @@ s390_valid_target_attribute_tree (tree args,
       || new_opts_set.x_s390_stack_size
       || new_opts_set.x_s390_branch_cost
       || new_opts_set.x_s390_warn_framesize
-      || new_opts_set.x_s390_warn_dynamicstack_p
-      )
+      || new_opts_set.x_s390_warn_dynamicstack_p)
     {
-      /* If we are using the default tune= or arch=, undo the value assigned,
-	 and use the default.  */
-      if (!new_opts_set.x_s390_arch)
-	{
-	  if (!orig_arch_specified)
-	    opts->x_s390_arch = PROCESSOR_max;
-	}
-      if (!new_opts_set.x_s390_tune && new_opts_set.x_s390_arch)
-	{
-	  if (opts->x_s390_arch != PROCESSOR_max)
-	    opts->x_s390_tune = PROCESSOR_max;
-	  else if (!orig_tune_specified)
-	    opts->x_s390_tune = PROCESSOR_max;
-	}
+      struct gcc_options joined_opts_set;
+      unsigned char *src = (unsigned char *)&new_opts_set;
+      unsigned char *dest = (unsigned char *)&joined_opts_set;
+      unsigned int i;
 
-      {
-	struct gcc_options joined_opts_set;
-	unsigned char *src = (unsigned char *)&new_opts_set;
-	unsigned char *dest = (unsigned char *)&joined_opts_set;
-	unsigned int i;
-
-	/* Make a joined copy of the original and the additional option flags.
-	 */
-	memcpy (&joined_opts_set, opts_set, sizeof (*opts_set));
-	for (i = 0; i < sizeof(*opts_set); i++)
-	  dest[i] |= src[i];
-
-	/* Do any overrides, such as arch=xxx, or tune=xxx support.  */
-	s390_option_override_internal (opts, &joined_opts_set);
-      }
+      /* A single -march overwrites a former -mtune.  */
+      if (new_opts_set.x_s390_arch && !new_opts_set.x_s390_tune)
+	opts_set->x_s390_tune = (enum processor_type)0;
 
+      /* Make a joined copy of the original and the additional option
+	 flags.  */
+      joined_opts_set = *opts_set;
+      for (i = 0; i < sizeof(*opts_set); i++)
+	dest[i] |= src[i];
+
+      /* Do any overrides, such as arch=xxx, or tune=xxx support.  */
+      s390_option_override_internal (opts, &joined_opts_set);
       /* Save the current options unless we are validating options for
 	 #pragma.  */
       t = build_target_option_node (opts);
     }
-
   return t;
 }
 
diff --git a/gcc/config/s390/s390.opt b/gcc/config/s390/s390.opt
index 4bddda5..409ec72 100644
--- a/gcc/config/s390/s390.opt
+++ b/gcc/config/s390/s390.opt
@@ -23,18 +23,10 @@ config/s390/s390-opts.h
 
 ;; Definitions to add to the cl_target_option and gcc_options structures
 
-;; whether -march was specified
-TargetVariable
-unsigned char s390_arch_specified
-
 ;; Flags derived from s390_arch
 TargetVariable
 int s390_arch_flags
 
-;; whether -mtune was specified
-TargetVariable
-unsigned char s390_tune_specified
-
 ;; Flags derived from s390_tune
 TargetVariable
 int s390_tune_flags
@@ -158,7 +150,7 @@ Target RejectNegative Joined UInteger Var(s390_stack_size) Save
 Emit extra code in the function prologue in order to trap if the stack size exceeds the given limit.
 
 mtune=
-Target RejectNegative Joined Enum(processor_type) Var(s390_tune) Init(PROCESSOR_max) Save.
+Target RejectNegative Joined Enum(processor_type) Var(s390_tune) Init(PROCESSOR_max) Save
 Schedule code for given CPU
 
 mmvcle