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

[Dominik Vogt <vogt at linux dot vnet dot ibm dot com>]

(@Uli: I'd like to hear your opinion on this issue.
Original message:
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03403.html).

On Fri, Oct 30, 2015 at 03:09:39PM +0100, Andreas Krebbel wrote:
> 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).

The idea was that -mtune on the command line is *not* overridden
by the "arch" target attribute.  This would allow to change the
architecture for a specific function and keep the -mtune= option
from the command line.  But as a matter of fact, the current patch
doesn't do it either (bug?).

Example:

  -- foo.c --
  __attribute__ ((target("arch=z1EC12")))
  void foo(void) { ... }
  -- foo.c --

  $ gcc -c -mtune=z13 foo.c -o foo-z13.c   # ==> tuned for zEC12
  $ gcc -c -mtune=z10 foo.c -o foo-z10.c   # ==> tuned for zEC12

If this requirement is dropped, it should be possible to get rid
of the s390_..._specified thing as it is not used for anything
else (it probably was in earlier patches).

(Your patch:) 
> +      /* 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;

We cannot just overwrite the old *opts structure.  There may not
be a backup of it ...

> +      /* 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];

... rather reset the bit in the joined structure.

      if (new_opts_set.x_s390_arch && !new_opts_set.x_s390_tune)
	joined_opts_set->x_s390_tune = new_opts_set->x_s390_tune;

(The opts_set parameter should really be const; I'll check if this
can be done.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany