This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]