[PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Peter Bergner]

The -mdejagnu-cpu= option was added as a way for a test case to ensure a
particular -mcpu= option is used to compile the test, regardless of whether
the user attempts to override it (purposely or accidentally) via RUNTESTFLAGS.
This was well and good, but the ASM_CPU_SPEC was not updated to handle
-mdejagnu-cpu=, so the option passed to the assembler is only determined
by -mcpu=, even if that is overridden by -mdejagnu-cpu=.  This can cause
cases of using the wrong assembler option.

We could just add -mdejagnu-cpu= support to ASM_CPU_SPEC, but that spec entry
is already WAY too huge and ugly and to add support for -mdejagnu-cpu=, we'd
have to essentially double the size of that spec.  The patch below takes
a different approach and removes Segher's original patch altogether and
instead implements -mdejagnu-cpu= using a DRIVER_SELF_SPECS spec which
simplifies things by not even needing to touch ASM_CPU_SPEC.  I also added
support for -mdejagnu-tune=, even though we don't have any test cases
at the moment that use it, since it was easy to add.

Segher, I tried your suggestion of writing the spec more generically
(ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
replacement.  However, the %<m... hunk to strip the overridden -mcpu=
option(s) would need to be written like %<m%* and that does not work
at all.

This passed bootstrap and regtesting with no errors on powerpc64le-linux.
Ok for trunk?

Peter

gcc/
	PR target/91050
	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
	use of deleted rs6000_dejagnu_cpu_index variable.
	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.

Index: gcc/config/rs6000/rs6000.opt
===================================================================
--- gcc/config/rs6000/rs6000.opt	(revision 273707)
+++ gcc/config/rs6000/rs6000.opt	(working copy)
@@ -388,13 +388,6 @@ mtune=
 Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
 -mtune=	Schedule code for given CPU.
 
-; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
-; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
-; override those set in the testcases; with this option, the testcase will
-; always win.
-mdejagnu-cpu=
-Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
-
 mtraceback=
 Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
 -mtraceback=[full,part,no]	Select type of traceback table.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 273707)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
   /* Don't override by the processor default if given explicitly.  */
   set_masks &= ~rs6000_isa_flags_explicit;
 
-  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
-    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
-
   /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
      the cpu in a target attribute or pragma, but did not specify a tuning
      option, use the cpu for the tuning option rather than the option specified
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 273707)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -77,6 +77,15 @@
 #define PPC405_ERRATUM77 0
 #endif
 
+/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
+   With older versions of Dejagnu the command line arguments you set in
+   RUNTESTFLAGS override those set in the testcases; with this option,
+   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
+#define DRIVER_SELF_SPECS \
+  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
+   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
+   %{mdejagnu-*: %<mdejagnu-*}"
+
 #if CHECKING_P
 #define ASM_OPT_ANY ""
 #else

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Iain Sandoe]

Hi Peter,

> On 25 Jul 2019, at 04:30, Peter Bergner <peter@bergner.org> wrote:
> 
> The -mdejagnu-cpu= option was added as a way for a test case to ensure a
> particular -mcpu= option is used to compile the test, regardless of whether
> the user attempts to override it (purposely or accidentally) via RUNTESTFLAGS.
> This was well and good, but the ASM_CPU_SPEC was not updated to handle
> -mdejagnu-cpu=, so the option passed to the assembler is only determined
> by -mcpu=, even if that is overridden by -mdejagnu-cpu=.  This can cause
> cases of using the wrong assembler option.
> 
> We could just add -mdejagnu-cpu= support to ASM_CPU_SPEC, but that spec entry
> is already WAY too huge and ugly and to add support for -mdejagnu-cpu=, we'd
> have to essentially double the size of that spec.  The patch below takes
> a different approach and removes Segher's original patch altogether and
> instead implements -mdejagnu-cpu= using a DRIVER_SELF_SPECS spec which
> simplifies things by not even needing to touch ASM_CPU_SPEC.  I also added
> support for -mdejagnu-tune=, even though we don't have any test cases
> at the moment that use it, since it was easy to add.

This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
since they were introduced (and the equivalent before).

This is because Darwin has driver self-specs common to the PPC and X86 ports.

If this change is seen the way to go, then I guess one solution would be to rename the
driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
put a dummy DRIVER_SELF_SPECS in i386/i386.h 
and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h 
cases.  (or something along those lines)

> Segher, I tried your suggestion of writing the spec more generically
> (ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
> replacement.  However, the %<m... hunk to strip the overridden -mcpu=
> option(s) would need to be written like %<m%* and that does not work
> at all.

not sure what the objective is here - if you want to remove all pre-existing -mcpu from
the command line won’t %<mcpu*  work for you? (with the risk of removing -mcpunewthing
if it gets invented) ..

thanks
Iain

*** it seems a bit odd that the upper directory contains the SUBTARGET designation, but
that’s a pattern used before.

> This passed bootstrap and regtesting with no errors on powerpc64le-linux.
> Ok for trunk?
> 
> Peter
> 
> gcc/
> 	PR target/91050
> 	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
> 	use of deleted rs6000_dejagnu_cpu_index variable.
> 	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
> 
> Index: gcc/config/rs6000/rs6000.opt
> ===================================================================
> --- gcc/config/rs6000/rs6000.opt	(revision 273707)
> +++ gcc/config/rs6000/rs6000.opt	(working copy)
> @@ -388,13 +388,6 @@ mtune=
> Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> -mtune=	Schedule code for given CPU.
> 
> -; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
> -; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
> -; override those set in the testcases; with this option, the testcase will
> -; always win.
> -mdejagnu-cpu=
> -Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> -
> mtraceback=
> Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
> -mtraceback=[full,part,no]	Select type of traceback table.
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 273707)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
>   /* Don't override by the processor default if given explicitly.  */
>   set_masks &= ~rs6000_isa_flags_explicit;
> 
> -  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
> -    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
> -
>   /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
>      the cpu in a target attribute or pragma, but did not specify a tuning
>      option, use the cpu for the tuning option rather than the option specified
> Index: gcc/config/rs6000/rs6000.h
> ===================================================================
> --- gcc/config/rs6000/rs6000.h	(revision 273707)
> +++ gcc/config/rs6000/rs6000.h	(working copy)
> @@ -77,6 +77,15 @@
> #define PPC405_ERRATUM77 0
> #endif
> 
> +/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
> +   With older versions of Dejagnu the command line arguments you set in
> +   RUNTESTFLAGS override those set in the testcases; with this option,
> +   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
> +#define DRIVER_SELF_SPECS \
> +  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
> +   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
> +   %{mdejagnu-*: %<mdejagnu-*}"
> +
> #if CHECKING_P
> #define ASM_OPT_ANY ""
> #else

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Peter Bergner]

On 7/25/19 2:50 AM, Iain Sandoe wrote:
> This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
> since they were introduced (and the equivalent before).
> 
> This is because Darwin has driver self-specs common to the PPC and X86 ports.
> 
> If this change is seen the way to go, then I guess one solution would be to rename the
> driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
> put a dummy DRIVER_SELF_SPECS in i386/i386.h 
> and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h 
> cases.  (or something along those lines)

Hi Iain,

The patch below uses your suggestion.  Does this work for you on x86 and ppc?



>> Segher, I tried your suggestion of writing the spec more generically
>> (ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
>> replacement.  However, the %<m... hunk to strip the overridden -mcpu=
>> option(s) would need to be written like %<m%* and that does not work
>> at all.
> 
> not sure what the objective is here - if you want to remove all pre-existing -mcpu from
> the command line won’t %<mcpu*  work for you? (with the risk of removing -mcpunewthing
> if it gets invented) ..

We only want to remove all pre-existing -mcpu= options IF the user also used
-mdejagnu-cpu=.  You do not want to remove -mcpu= options if the user used
-mdejagnu-tune=.  That's why I tried:

    %{mdejagnu-*: %<m%* -m*}

so -mdejagnu-cpu= would only remove -mcpu options and -mdejagnu-tune= would
only remove -mtune= options.  The problem is that it doesn't look like you
can you use %* with %<.

Peter


gcc/
	PR target/91050
	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
	use of deleted rs6000_dejagnu_cpu_index variable.
	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
	* config/darwin.h (DRIVER_SELF_SPECS): Rename from this ...
	(SUBTARGET_DRIVER_SELF_SPECS): ...to this.
	* config/i386/i386.h (DRIVER_SELF_SPECS): Define.
	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.

Index: gcc/config/rs6000/rs6000.opt
===================================================================
--- gcc/config/rs6000/rs6000.opt	(revision 273707)
+++ gcc/config/rs6000/rs6000.opt	(working copy)
@@ -388,13 +388,6 @@ mtune=
 Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
 -mtune=	Schedule code for given CPU.
 
-; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
-; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
-; override those set in the testcases; with this option, the testcase will
-; always win.
-mdejagnu-cpu=
-Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
-
 mtraceback=
 Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
 -mtraceback=[full,part,no]	Select type of traceback table.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 273707)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
   /* Don't override by the processor default if given explicitly.  */
   set_masks &= ~rs6000_isa_flags_explicit;
 
-  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
-    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
-
   /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
      the cpu in a target attribute or pragma, but did not specify a tuning
      option, use the cpu for the tuning option rather than the option specified
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 273707)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -77,6 +77,20 @@
 #define PPC405_ERRATUM77 0
 #endif
 
+/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
+   With older versions of Dejagnu the command line arguments you set in
+   RUNTESTFLAGS override those set in the testcases; with this option,
+   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
+#define DRIVER_SELF_SPECS \
+  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
+   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
+   %{mdejagnu-*: %<mdejagnu-*}" \
+   SUBTARGET_DRIVER_SELF_SPECS
+
+#ifndef SUBTARGET_DRIVER_SELF_SPECS
+# define SUBTARGET_DRIVER_SELF_SPECS
+#endif
+
 #if CHECKING_P
 #define ASM_OPT_ANY ""
 #else
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 273707)
+++ gcc/config/darwin.h	(working copy)
@@ -125,7 +125,7 @@ extern GTY(()) int darwin_ms_struct;
 
    However, a few can be handled and we can elide options that are silently-
    ignored defaults, plus warn on obsolete ones that no longer function.  */
-#define DRIVER_SELF_SPECS						\
+#define SUBTARGET_DRIVER_SELF_SPECS					\
 "%{fapple-kext|mkernel:-static}",					\
 "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",		\
 "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \
Index: gcc/config/i386/i386.h
===================================================================
--- gcc/config/i386/i386.h	(revision 273707)
+++ gcc/config/i386/i386.h	(working copy)
@@ -677,6 +677,12 @@ extern tree x86_mfence;
    with the rounding mode forced to 53 bits.  */
 #define TARGET_96_ROUND_53_LONG_DOUBLE 0
 
+#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
+
+#ifndef SUBTARGET_DRIVER_SELF_SPECS
+# define SUBTARGET_DRIVER_SELF_SPECS
+#endif
+
 /* -march=native handling only makes sense with compiler running on
    an x86 or x86_64 chip.  If changing this condition, also change
    the condition in driver-i386.c.  */

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Iain Sandoe]

Hi Peter,

> On 25 Jul 2019, at 15:41, Peter Bergner <bergner@linux.ibm.com> wrote:
> 
> On 7/25/19 2:50 AM, Iain Sandoe wrote:
>> This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
>> since they were introduced (and the equivalent before).
>> 
>> This is because Darwin has driver self-specs common to the PPC and X86 ports.
>> 
>> If this change is seen the way to go, then I guess one solution would be to rename the
>> driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
>> put a dummy DRIVER_SELF_SPECS in i386/i386.h 
>> and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h 
>> cases.  (or something along those lines)

> The patch below uses your suggestion.  Does this work for you on x86 and ppc?

It’s in the queue for testing tonight - my ppc h/w is somewhat slow, 

thanks for handling this,
Iain

> 
>>> Segher, I tried your suggestion of writing the spec more generically
>>> (ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
>>> replacement.  However, the %<m... hunk to strip the overridden -mcpu=
>>> option(s) would need to be written like %<m%* and that does not work
>>> at all.
>> 
>> not sure what the objective is here - if you want to remove all pre-existing -mcpu from
>> the command line won’t %<mcpu*  work for you? (with the risk of removing -mcpunewthing
>> if it gets invented) ..
> 
> We only want to remove all pre-existing -mcpu= options IF the user also used
> -mdejagnu-cpu=.  You do not want to remove -mcpu= options if the user used
> -mdejagnu-tune=.  That's why I tried:
> 
>    %{mdejagnu-*: %<m%* -m*}
> 
> so -mdejagnu-cpu= would only remove -mcpu options and -mdejagnu-tune= would
> only remove -mtune= options.  The problem is that it doesn't look like you
> can you use %* with %<.
> 
> Peter
> 
> 
> gcc/
> 	PR target/91050
> 	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
> 	use of deleted rs6000_dejagnu_cpu_index variable.
> 	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
> 	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
> 	* config/darwin.h (DRIVER_SELF_SPECS): Rename from this ...
> 	(SUBTARGET_DRIVER_SELF_SPECS): ...to this.
> 	* config/i386/i386.h (DRIVER_SELF_SPECS): Define.
> 	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
> 
> Index: gcc/config/rs6000/rs6000.opt
> ===================================================================
> --- gcc/config/rs6000/rs6000.opt	(revision 273707)
> +++ gcc/config/rs6000/rs6000.opt	(working copy)
> @@ -388,13 +388,6 @@ mtune=
> Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> -mtune=	Schedule code for given CPU.
> 
> -; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
> -; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
> -; override those set in the testcases; with this option, the testcase will
> -; always win.
> -mdejagnu-cpu=
> -Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> -
> mtraceback=
> Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
> -mtraceback=[full,part,no]	Select type of traceback table.
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 273707)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
>   /* Don't override by the processor default if given explicitly.  */
>   set_masks &= ~rs6000_isa_flags_explicit;
> 
> -  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
> -    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
> -
>   /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
>      the cpu in a target attribute or pragma, but did not specify a tuning
>      option, use the cpu for the tuning option rather than the option specified
> Index: gcc/config/rs6000/rs6000.h
> ===================================================================
> --- gcc/config/rs6000/rs6000.h	(revision 273707)
> +++ gcc/config/rs6000/rs6000.h	(working copy)
> @@ -77,6 +77,20 @@
> #define PPC405_ERRATUM77 0
> #endif
> 
> +/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
> +   With older versions of Dejagnu the command line arguments you set in
> +   RUNTESTFLAGS override those set in the testcases; with this option,
> +   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
> +#define DRIVER_SELF_SPECS \
> +  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
> +   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
> +   %{mdejagnu-*: %<mdejagnu-*}" \
> +   SUBTARGET_DRIVER_SELF_SPECS
> +
> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> +# define SUBTARGET_DRIVER_SELF_SPECS
> +#endif
> +
> #if CHECKING_P
> #define ASM_OPT_ANY ""
> #else
> Index: gcc/config/darwin.h
> ===================================================================
> --- gcc/config/darwin.h	(revision 273707)
> +++ gcc/config/darwin.h	(working copy)
> @@ -125,7 +125,7 @@ extern GTY(()) int darwin_ms_struct;
> 
>    However, a few can be handled and we can elide options that are silently-
>    ignored defaults, plus warn on obsolete ones that no longer function.  */
> -#define DRIVER_SELF_SPECS						\
> +#define SUBTARGET_DRIVER_SELF_SPECS					\
> "%{fapple-kext|mkernel:-static}",					\
> "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",		\
> "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \
> Index: gcc/config/i386/i386.h
> ===================================================================
> --- gcc/config/i386/i386.h	(revision 273707)
> +++ gcc/config/i386/i386.h	(working copy)
> @@ -677,6 +677,12 @@ extern tree x86_mfence;
>    with the rounding mode forced to 53 bits.  */
> #define TARGET_96_ROUND_53_LONG_DOUBLE 0
> 
> +#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
> +
> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> +# define SUBTARGET_DRIVER_SELF_SPECS
> +#endif
> +
> /* -march=native handling only makes sense with compiler running on
>    an x86 or x86_64 chip.  If changing this condition, also change
>    the condition in driver-i386.c.  */
> 

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Segher Boessenkool]

Hi Peter,

On Thu, Jul 25, 2019 at 09:41:10AM -0500, Peter Bergner wrote:
> On 7/25/19 2:50 AM, Iain Sandoe wrote:
> > This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
> > since they were introduced (and the equivalent before).
> > 
> > This is because Darwin has driver self-specs common to the PPC and X86 ports.
> > 
> > If this change is seen the way to go, then I guess one solution would be to rename the
> > driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
> > put a dummy DRIVER_SELF_SPECS in i386/i386.h 
> > and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h 
> > cases.  (or something along those lines)
> 
> Hi Iain,
> 
> The patch below uses your suggestion.  Does this work for you on x86 and ppc?

It all looks fine to me; if it works for Iain, it is approved for trunk,
and thank you!


Segher

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Peter Bergner]

Uros and Jan,

I should have CC'd you for the i386 portion of the patch.  Are you ok with
the i386.h change if Iain's x86 Darwin testing comes out clean?

Peter


On 7/25/19 9:41 AM, Peter Bergner wrote:
> On 7/25/19 2:50 AM, Iain Sandoe wrote:
>> This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
>> since they were introduced (and the equivalent before).
>>
>> This is because Darwin has driver self-specs common to the PPC and X86 ports.
>>
>> If this change is seen the way to go, then I guess one solution would be to rename the
>> driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
>> put a dummy DRIVER_SELF_SPECS in i386/i386.h 
>> and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h 
>> cases.  (or something along those lines)
> 
> Hi Iain,
> 
> The patch below uses your suggestion.  Does this work for you on x86 and ppc?
> 
> 
> 
>>> Segher, I tried your suggestion of writing the spec more generically
>>> (ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
>>> replacement.  However, the %<m... hunk to strip the overridden -mcpu=
>>> option(s) would need to be written like %<m%* and that does not work
>>> at all.
>>
>> not sure what the objective is here - if you want to remove all pre-existing -mcpu from
>> the command line won’t %<mcpu*  work for you? (with the risk of removing -mcpunewthing
>> if it gets invented) ..
> 
> We only want to remove all pre-existing -mcpu= options IF the user also used
> -mdejagnu-cpu=.  You do not want to remove -mcpu= options if the user used
> -mdejagnu-tune=.  That's why I tried:
> 
>     %{mdejagnu-*: %<m%* -m*}
> 
> so -mdejagnu-cpu= would only remove -mcpu options and -mdejagnu-tune= would
> only remove -mtune= options.  The problem is that it doesn't look like you
> can you use %* with %<.
> 
> Peter
> 
> 
> gcc/
> 	PR target/91050
> 	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
> 	use of deleted rs6000_dejagnu_cpu_index variable.
> 	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
> 	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
> 	* config/darwin.h (DRIVER_SELF_SPECS): Rename from this ...
> 	(SUBTARGET_DRIVER_SELF_SPECS): ...to this.
> 	* config/i386/i386.h (DRIVER_SELF_SPECS): Define.
> 	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
> 
> Index: gcc/config/rs6000/rs6000.opt
> ===================================================================
> --- gcc/config/rs6000/rs6000.opt	(revision 273707)
> +++ gcc/config/rs6000/rs6000.opt	(working copy)
> @@ -388,13 +388,6 @@ mtune=
>  Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
>  -mtune=	Schedule code for given CPU.
>  
> -; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
> -; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
> -; override those set in the testcases; with this option, the testcase will
> -; always win.
> -mdejagnu-cpu=
> -Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> -
>  mtraceback=
>  Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
>  -mtraceback=[full,part,no]	Select type of traceback table.
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 273707)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
>    /* Don't override by the processor default if given explicitly.  */
>    set_masks &= ~rs6000_isa_flags_explicit;
>  
> -  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
> -    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
> -
>    /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
>       the cpu in a target attribute or pragma, but did not specify a tuning
>       option, use the cpu for the tuning option rather than the option specified
> Index: gcc/config/rs6000/rs6000.h
> ===================================================================
> --- gcc/config/rs6000/rs6000.h	(revision 273707)
> +++ gcc/config/rs6000/rs6000.h	(working copy)
> @@ -77,6 +77,20 @@
>  #define PPC405_ERRATUM77 0
>  #endif
>  
> +/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
> +   With older versions of Dejagnu the command line arguments you set in
> +   RUNTESTFLAGS override those set in the testcases; with this option,
> +   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
> +#define DRIVER_SELF_SPECS \
> +  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
> +   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
> +   %{mdejagnu-*: %<mdejagnu-*}" \
> +   SUBTARGET_DRIVER_SELF_SPECS
> +
> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> +# define SUBTARGET_DRIVER_SELF_SPECS
> +#endif
> +
>  #if CHECKING_P
>  #define ASM_OPT_ANY ""
>  #else
> Index: gcc/config/darwin.h
> ===================================================================
> --- gcc/config/darwin.h	(revision 273707)
> +++ gcc/config/darwin.h	(working copy)
> @@ -125,7 +125,7 @@ extern GTY(()) int darwin_ms_struct;
>  
>     However, a few can be handled and we can elide options that are silently-
>     ignored defaults, plus warn on obsolete ones that no longer function.  */
> -#define DRIVER_SELF_SPECS						\
> +#define SUBTARGET_DRIVER_SELF_SPECS					\
>  "%{fapple-kext|mkernel:-static}",					\
>  "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",		\
>  "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \
> Index: gcc/config/i386/i386.h
> ===================================================================
> --- gcc/config/i386/i386.h	(revision 273707)
> +++ gcc/config/i386/i386.h	(working copy)
> @@ -677,6 +677,12 @@ extern tree x86_mfence;
>     with the rounding mode forced to 53 bits.  */
>  #define TARGET_96_ROUND_53_LONG_DOUBLE 0
>  
> +#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
> +
> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> +# define SUBTARGET_DRIVER_SELF_SPECS
> +#endif
> +
>  /* -march=native handling only makes sense with compiler running on
>     an x86 or x86_64 chip.  If changing this condition, also change
>     the condition in driver-i386.c.  */
> 

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Iain Sandoe]

Hi Peter,

The patch needs one small amendment  to succeed in bootstrap.
I applied the amended to i686, x86_64 and powerpc Darwin with no apparent
new problems.  From the Darwin perspective, the patch is OK with the 
amendment.

(there’s also a note in the text, but that’s just an observation)

thanks
Iain

> On 25 Jul 2019, at 18:34, Peter Bergner <bergner@linux.ibm.com> wrote:
> 
> Uros and Jan,
> 
> I should have CC'd you for the i386 portion of the patch.  Are you ok with
> the i386.h change if Iain's x86 Darwin testing comes out clean?
> 

> On 7/25/19 9:41 AM, Peter Bergner wrote:
>> On 7/25/19 2:50 AM, Iain Sandoe wrote:
>>> This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
>>> since they were introduced (and the equivalent before).
>>> 
>>> This is because Darwin has driver self-specs common to the PPC and X86 ports.
>>> 
>>> If this change is seen the way to go, then I guess one solution would be to rename the
>>> driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
>>> put a dummy DRIVER_SELF_SPECS in i386/i386.h 
>>> and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h 
>>> cases.  (or something along those lines)
>> 
>> 

>>>> Segher, I tried your suggestion of writing the spec more generically
>>>> (ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
>>>> replacement.  However, the %<m... hunk to strip the overridden -mcpu=
>>>> option(s) would need to be written like %<m%* and that does not work
>>>> at all.
>>> 
>>> not sure what the objective is here - if you want to remove all pre-existing -mcpu from
>>> the command line won’t %<mcpu*  work for you? (with the risk of removing -mcpunewthing
>>> if it gets invented) ..
>> 
>> We only want to remove all pre-existing -mcpu= options IF the user also used
>> -mdejagnu-cpu=.  You do not want to remove -mcpu= options if the user used
>> -mdejagnu-tune=.  That's why I tried:
>> 
>>    %{mdejagnu-*: %<m%* -m*}
>> 
>> so -mdejagnu-cpu= would only remove -mcpu options and -mdejagnu-tune= would
>> only remove -mtune= options.  The problem is that it doesn't look like you
>> can you use %* with %<.
>> 
>> Peter
>> 
>> 
>> gcc/
>> 	PR target/91050
>> 	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
>> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
>> 	use of deleted rs6000_dejagnu_cpu_index variable.
>> 	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
>> 	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
>> 	* config/darwin.h (DRIVER_SELF_SPECS): Rename from this ...
>> 	(SUBTARGET_DRIVER_SELF_SPECS): ...to this.
>> 	* config/i386/i386.h (DRIVER_SELF_SPECS): Define.
>> 	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
>> 
>> Index: gcc/config/rs6000/rs6000.opt
>> ===================================================================
>> --- gcc/config/rs6000/rs6000.opt	(revision 273707)
>> +++ gcc/config/rs6000/rs6000.opt	(working copy)
>> @@ -388,13 +388,6 @@ mtune=
>> Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
>> -mtune=	Schedule code for given CPU.
>> 
>> -; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
>> -; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
>> -; override those set in the testcases; with this option, the testcase will
>> -; always win.
>> -mdejagnu-cpu=
>> -Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
>> -
>> mtraceback=
>> Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
>> -mtraceback=[full,part,no]	Select type of traceback table.
>> Index: gcc/config/rs6000/rs6000.c
>> ===================================================================
>> --- gcc/config/rs6000/rs6000.c	(revision 273707)
>> +++ gcc/config/rs6000/rs6000.c	(working copy)
>> @@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
>>   /* Don't override by the processor default if given explicitly.  */
>>   set_masks &= ~rs6000_isa_flags_explicit;
>> 
>> -  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
>> -    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
>> -
>>   /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
>>      the cpu in a target attribute or pragma, but did not specify a tuning
>>      option, use the cpu for the tuning option rather than the option specified
>> Index: gcc/config/rs6000/rs6000.h
>> ===================================================================
>> --- gcc/config/rs6000/rs6000.h	(revision 273707)
>> +++ gcc/config/rs6000/rs6000.h	(working copy)
>> @@ -77,6 +77,20 @@
>> #define PPC405_ERRATUM77 0
>> #endif
>> 
>> +/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
>> +   With older versions of Dejagnu the command line arguments you set in
>> +   RUNTESTFLAGS override those set in the testcases; with this option,
>> +   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
>> +#define DRIVER_SELF_SPECS \
>> +  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
>> +   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
>> +   %{mdejagnu-*: %<mdejagnu-*}" \
>> +   SUBTARGET_DRIVER_SELF_SPECS

NOTE (only a note): most of the driver self-specs that are more than a single line
seem to be written as comma-separated strings each containing a single spec.
I am not sure what material difference that makes, and it doesn’t seem to affect
the functionality of your patch.  I also  asked Joseph on IRC yesterday and he had
no specific advice.

>> +
>> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
>> +# define SUBTARGET_DRIVER_SELF_SPECS
>> +#endif
>> +
>> #if CHECKING_P
>> #define ASM_OPT_ANY ""
>> #else
>> Index: gcc/config/darwin.h
>> ===================================================================
>> --- gcc/config/darwin.h	(revision 273707)
>> +++ gcc/config/darwin.h	(working copy)
>> @@ -125,7 +125,7 @@ extern GTY(()) int darwin_ms_struct;
>> 
>>    However, a few can be handled and we can elide options that are silently-
>>    ignored defaults, plus warn on obsolete ones that no longer function.  */
>> -#define DRIVER_SELF_SPECS						\
>> +#define SUBTARGET_DRIVER_SELF_SPECS					\
>> "%{fapple-kext|mkernel:-static}",					\
>> "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",		\
>> "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \
>> Index: gcc/config/i386/i386.h

this needs to be:

diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index ed8798472a..ab5ad03f17 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -125,7 +125,8 @@ extern GTY(()) int darwin_ms_struct;
 
    However, a few can be handled and we can elide options that are silently-
    ignored defaults, plus warn on obsolete ones that no longer function.  */
-#define DRIVER_SELF_SPECS                                              \
+#undef SUBTARGET_DRIVER_SELF_SPECS
+#define SUBTARGET_DRIVER_SELF_SPECS                                    \
 "%{fapple-kext|mkernel:-static}",                                      \
 "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",             \
 "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \


>> ===================================================================
>> --- gcc/config/i386/i386.h	(revision 273707)
>> +++ gcc/config/i386/i386.h	(working copy)
>> @@ -677,6 +677,12 @@ extern tree x86_mfence;
>>    with the rounding mode forced to 53 bits.  */
>> #define TARGET_96_ROUND_53_LONG_DOUBLE 0
>> 
>> +#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
>> +
>> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
>> +# define SUBTARGET_DRIVER_SELF_SPECS
>> +#endif
>> +
>> /* -march=native handling only makes sense with compiler running on
>>    an x86 or x86_64 chip.  If changing this condition, also change
>>    the condition in driver-i386.c.  */
>> 
> 

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Uros Bizjak]

On Thu, Jul 25, 2019 at 7:34 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>
> Uros and Jan,
>
> I should have CC'd you for the i386 portion of the patch.  Are you ok with
> the i386.h change if Iain's x86 Darwin testing comes out clean?
>
> Peter
>
>
> On 7/25/19 9:41 AM, Peter Bergner wrote:
> > On 7/25/19 2:50 AM, Iain Sandoe wrote:
> >> This will break Darwin which has used DRIVER_SELF_SPECS in config/darwin.h
> >> since they were introduced (and the equivalent before).
> >>
> >> This is because Darwin has driver self-specs common to the PPC and X86 ports.
> >>
> >> If this change is seen the way to go, then I guess one solution would be to rename the
> >> driver specs in config/darwin.h => SUBTARGET_DRIVER_SELF_SPECS*** and then
> >> put a dummy DRIVER_SELF_SPECS in i386/i386.h
> >> and append SUBTARGET_DRIVER_SELF_SPECS  to both the i386.h and rs6000.h
> >> cases.  (or something along those lines)
> >
> > Hi Iain,
> >
> > The patch below uses your suggestion.  Does this work for you on x86 and ppc?
> >
> >
> >
> >>> Segher, I tried your suggestion of writing the spec more generically
> >>> (ie, %{mdejagnu-*: ... -m*}), which worked in that it did the correct
> >>> replacement.  However, the %<m... hunk to strip the overridden -mcpu=
> >>> option(s) would need to be written like %<m%* and that does not work
> >>> at all.
> >>
> >> not sure what the objective is here - if you want to remove all pre-existing -mcpu from
> >> the command line won’t %<mcpu*  work for you? (with the risk of removing -mcpunewthing
> >> if it gets invented) ..
> >
> > We only want to remove all pre-existing -mcpu= options IF the user also used
> > -mdejagnu-cpu=.  You do not want to remove -mcpu= options if the user used
> > -mdejagnu-tune=.  That's why I tried:
> >
> >     %{mdejagnu-*: %<m%* -m*}
> >
> > so -mdejagnu-cpu= would only remove -mcpu options and -mdejagnu-tune= would
> > only remove -mtune= options.  The problem is that it doesn't look like you
> > can you use %* with %<.
> >
> > Peter
> >
> >
> > gcc/
> >       PR target/91050
> >       * config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
> >       * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
> >       use of deleted rs6000_dejagnu_cpu_index variable.
> >       * config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
> >       (SUBTARGET_DRIVER_SELF_SPECS): Likewise.
> >       * config/darwin.h (DRIVER_SELF_SPECS): Rename from this ...
> >       (SUBTARGET_DRIVER_SELF_SPECS): ...to this.
> >       * config/i386/i386.h (DRIVER_SELF_SPECS): Define.
> >       (SUBTARGET_DRIVER_SELF_SPECS): Likewise.
> >
> > Index: gcc/config/rs6000/rs6000.opt
> > ===================================================================
> > --- gcc/config/rs6000/rs6000.opt      (revision 273707)
> > +++ gcc/config/rs6000/rs6000.opt      (working copy)
> > @@ -388,13 +388,6 @@ mtune=
> >  Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> >  -mtune=      Schedule code for given CPU.
> >
> > -; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
> > -; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
> > -; override those set in the testcases; with this option, the testcase will
> > -; always win.
> > -mdejagnu-cpu=
> > -Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
> > -
> >  mtraceback=
> >  Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
> >  -mtraceback=[full,part,no]   Select type of traceback table.
> > Index: gcc/config/rs6000/rs6000.c
> > ===================================================================
> > --- gcc/config/rs6000/rs6000.c        (revision 273707)
> > +++ gcc/config/rs6000/rs6000.c        (working copy)
> > @@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
> >    /* Don't override by the processor default if given explicitly.  */
> >    set_masks &= ~rs6000_isa_flags_explicit;
> >
> > -  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
> > -    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
> > -
> >    /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
> >       the cpu in a target attribute or pragma, but did not specify a tuning
> >       option, use the cpu for the tuning option rather than the option specified
> > Index: gcc/config/rs6000/rs6000.h
> > ===================================================================
> > --- gcc/config/rs6000/rs6000.h        (revision 273707)
> > +++ gcc/config/rs6000/rs6000.h        (working copy)
> > @@ -77,6 +77,20 @@
> >  #define PPC405_ERRATUM77 0
> >  #endif
> >
> > +/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
> > +   With older versions of Dejagnu the command line arguments you set in
> > +   RUNTESTFLAGS override those set in the testcases; with this option,
> > +   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
> > +#define DRIVER_SELF_SPECS \
> > +  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
> > +   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
> > +   %{mdejagnu-*: %<mdejagnu-*}" \
> > +   SUBTARGET_DRIVER_SELF_SPECS
> > +
> > +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> > +# define SUBTARGET_DRIVER_SELF_SPECS
> > +#endif
> > +
> >  #if CHECKING_P
> >  #define ASM_OPT_ANY ""
> >  #else
> > Index: gcc/config/darwin.h
> > ===================================================================
> > --- gcc/config/darwin.h       (revision 273707)
> > +++ gcc/config/darwin.h       (working copy)
> > @@ -125,7 +125,7 @@ extern GTY(()) int darwin_ms_struct;
> >
> >     However, a few can be handled and we can elide options that are silently-
> >     ignored defaults, plus warn on obsolete ones that no longer function.  */
> > -#define DRIVER_SELF_SPECS                                            \
> > +#define SUBTARGET_DRIVER_SELF_SPECS                                  \
> >  "%{fapple-kext|mkernel:-static}",                                    \
> >  "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",           \
> >  "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \
> > Index: gcc/config/i386/i386.h
> > ===================================================================
> > --- gcc/config/i386/i386.h    (revision 273707)
> > +++ gcc/config/i386/i386.h    (working copy)
> > @@ -677,6 +677,12 @@ extern tree x86_mfence;
> >     with the rounding mode forced to 53 bits.  */
> >  #define TARGET_96_ROUND_53_LONG_DOUBLE 0
> >
> > +#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
> > +
> > +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> > +# define SUBTARGET_DRIVER_SELF_SPECS
> > +#endif

Shouldn't we swap these two defines, so we always get SUBTARGET_D_S_S
defined first? Like:

#ifndef SUBTARGET_DRIVER_SELF_SPECS
# define SUBTARGET_DRIVER_SELF_SPECS ""
#endif

#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS

Uros.

> >  /* -march=native handling only makes sense with compiler running on
> >     an x86 or x86_64 chip.  If changing this condition, also change
> >     the condition in driver-i386.c.  */
> >
>

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Peter Bergner]

On 7/30/19 7:52 AM, Uros Bizjak wrote:
> On Thu, Jul 25, 2019 at 7:34 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>>> +#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
>>> +
>>> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
>>> +# define SUBTARGET_DRIVER_SELF_SPECS
>>> +#endif
> 
> Shouldn't we swap these two defines, so we always get SUBTARGET_D_S_S
> defined first? Like:
> 
> #ifndef SUBTARGET_DRIVER_SELF_SPECS
> # define SUBTARGET_DRIVER_SELF_SPECS ""
> #endif
> 
> #define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS

That's the way I would have coded it myself, but when I looked at the
other arches handling of spec defines, they all seemed to code it this
way.  Consider it swapped.

Peter

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Uros Bizjak]

On Tue, Jul 30, 2019 at 5:33 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>
> On 7/30/19 7:52 AM, Uros Bizjak wrote:
> > On Thu, Jul 25, 2019 at 7:34 PM Peter Bergner <bergner@linux.ibm.com> wrote:
> >>> +#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
> >>> +
> >>> +#ifndef SUBTARGET_DRIVER_SELF_SPECS
> >>> +# define SUBTARGET_DRIVER_SELF_SPECS
> >>> +#endif
> >
> > Shouldn't we swap these two defines, so we always get SUBTARGET_D_S_S
> > defined first? Like:
> >
> > #ifndef SUBTARGET_DRIVER_SELF_SPECS
> > # define SUBTARGET_DRIVER_SELF_SPECS ""
> > #endif
> >
> > #define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
>
> That's the way I would have coded it myself, but when I looked at the
> other arches handling of spec defines, they all seemed to code it this
> way.  Consider it swapped.

Yes, this looks much more readable to me; consider the x86 part OK.

Uros.

Re: [PATCH, rs6000] Fix PR91050 by adding a DRIVER_SELF_SPECS spec

[Peter Bergner]

On 7/26/19 9:28 AM, Iain Sandoe wrote:
> The patch needs one small amendment  to succeed in bootstrap.
> I applied the amended to i686, x86_64 and powerpc Darwin with no apparent
> new problems.  From the Darwin perspective, the patch is OK with the 
> amendment.

Ok, thanks.  For completeness, I have included the final patch that I
committed below, incorporating your and Uros' suggested changes.
Thank you both!



>>> +#define DRIVER_SELF_SPECS \
>>> +  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*} \
>>> +   %{mdejagnu-tune=*: %<mtune=* -mtune=%*} \
>>> +   %{mdejagnu-*: %<mdejagnu-*}" \
>>> +   SUBTARGET_DRIVER_SELF_SPECS
> 
> NOTE (only a note): most of the driver self-specs that are more than a single line
> seem to be written as comma-separated strings each containing a single spec.
> I am not sure what material difference that makes, and it doesn’t seem to affect
> the functionality of your patch.  I also  asked Joseph on IRC yesterday and he had
> no specific advice.

I see them used both ways, with and without the ','.  Playing around a little,
I see if you use ',' between clauses, then you also need to have each clause within
its own set of '"'s.  So "foo", "bar", "baz" etc. is fine, as is "foo bar baz", but
"foo, bar, baz" is not ok.  I guess it must have something to do with how we
scan the spec string.  Anyway, since the darwin SUBTARGET_DRIVER_SELF_SPECS
code uses ','s, I rewrote the rs6000.h DRIVER_SELF_SPECS that way too, so that
they're similar.

Peter

	PR target/91050
	* config/rs6000/rs6000.opt (mdejagnu-cpu=): Delete option.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
	use of deleted rs6000_dejagnu_cpu_index variable.
	* config/rs6000/rs6000.h (DRIVER_SELF_SPECS): Define.
	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.
	* config/darwin.h (DRIVER_SELF_SPECS): Rename from this ...
	(SUBTARGET_DRIVER_SELF_SPECS): ...to this.
	* config/i386/i386.h (DRIVER_SELF_SPECS): Define.
	(SUBTARGET_DRIVER_SELF_SPECS): Likewise.

Index: gcc/config/rs6000/rs6000.opt
===================================================================
--- gcc/config/rs6000/rs6000.opt	(revision 273940)
+++ gcc/config/rs6000/rs6000.opt	(revision 273941)
@@ -388,13 +388,6 @@ mtune=
 Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
 -mtune=	Schedule code for given CPU.
 
-; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
-; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
-; override those set in the testcases; with this option, the testcase will
-; always win.
-mdejagnu-cpu=
-Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) Init(-1) Enum(rs6000_cpu_opt_value) Save
-
 mtraceback=
 Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
 -mtraceback=[full,part,no]	Select type of traceback table.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 273940)
+++ gcc/config/rs6000/rs6000.c	(revision 273941)
@@ -3489,9 +3489,6 @@ rs6000_option_override_internal (bool gl
   /* Don't override by the processor default if given explicitly.  */
   set_masks &= ~rs6000_isa_flags_explicit;
 
-  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
-    rs6000_cpu_index = rs6000_dejagnu_cpu_index;
-
   /* Process the -mcpu=<xxx> and -mtune=<xxx> argument.  If the user changed
      the cpu in a target attribute or pragma, but did not specify a tuning
      option, use the cpu for the tuning option rather than the option specified
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 273940)
+++ gcc/config/rs6000/rs6000.h	(revision 273941)
@@ -77,6 +77,20 @@
 #define PPC405_ERRATUM77 0
 #endif
 
+#ifndef SUBTARGET_DRIVER_SELF_SPECS
+# define SUBTARGET_DRIVER_SELF_SPECS ""
+#endif
+
+/* Only for use in the testsuite: -mdejagnu-cpu= simply overrides -mcpu=.
+   With older versions of Dejagnu the command line arguments you set in
+   RUNTESTFLAGS override those set in the testcases; with this option,
+   the testcase will always win.  Ditto for -mdejagnu-tune=.  */
+#define DRIVER_SELF_SPECS \
+  "%{mdejagnu-cpu=*: %<mcpu=* -mcpu=%*}", \
+  "%{mdejagnu-tune=*: %<mtune=* -mtune=%*}", \
+  "%{mdejagnu-*: %<mdejagnu-*}", \
+   SUBTARGET_DRIVER_SELF_SPECS
+
 #if CHECKING_P
 #define ASM_OPT_ANY ""
 #else
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 273940)
+++ gcc/config/darwin.h	(revision 273941)
@@ -125,7 +125,8 @@ extern GTY(()) int darwin_ms_struct;
 
    However, a few can be handled and we can elide options that are silently-
    ignored defaults, plus warn on obsolete ones that no longer function.  */
-#define DRIVER_SELF_SPECS						\
+#undef SUBTARGET_DRIVER_SELF_SPECS
+#define SUBTARGET_DRIVER_SELF_SPECS					\
 "%{fapple-kext|mkernel:-static}",					\
 "%{gfull:-g -fno-eliminate-unused-debug-symbols} %<gfull",		\
 "%{gsplit-dwarf:%ngsplit-dwarf is not supported on this platform} \
Index: gcc/config/i386/i386.h
===================================================================
--- gcc/config/i386/i386.h	(revision 273940)
+++ gcc/config/i386/i386.h	(revision 273941)
@@ -677,6 +677,12 @@ extern tree x86_mfence;
    with the rounding mode forced to 53 bits.  */
 #define TARGET_96_ROUND_53_LONG_DOUBLE 0
 
+#ifndef SUBTARGET_DRIVER_SELF_SPECS
+# define SUBTARGET_DRIVER_SELF_SPECS ""
+#endif
+
+#define DRIVER_SELF_SPECS SUBTARGET_DRIVER_SELF_SPECS
+
 /* -march=native handling only makes sense with compiler running on
    an x86 or x86_64 chip.  If changing this condition, also change
    the condition in driver-i386.c.  */