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 <berg...@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. */ >> >