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

Reply via email to