Hi! On Mon, Apr 06, 2020 at 12:52:04PM -0400, Michael Meissner wrote: > Commit message: > Enable -mpcrel for -mcpu=future if it is allowed by the ABI.
You can just put this in the mail subject. No dot at the end, but start with a capital, an put a subsystem label on it: Subject: rs6000: Enable -mpcrel for -mcpu=future if it is allowed by the ABI (As you see, it is a bit long already). > 2020-04-06 Michael Meissner <meiss...@linux.ibm.com> > > * config/rs6000/linux64.h (PCREL_SUPPORTED_BY_OS): Enable > prefixed PC-relative addressing if the ABI supports it. > * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do not > set OPTION_MASK_PREFIXED here. > * config/rs6000/rs6000.c (rs6000_option_override_internal): Enable > OPTION_MASK_PREFIXED and OPTION_MASK_PCREL on -mcpu=future by > default if the current ABI allows the options. The changelog goes at the *end* of the commit message. Oh, there was nothing more? > --- /tmp/XzRKno_rs6000-cpus.def 2020-04-03 17:15:05.068676928 -0400 > +++ gcc/config/rs6000/rs6000-cpus.def 2020-04-03 17:00:50.115550614 -0400 > @@ -75,10 +75,14 @@ > | OPTION_MASK_P8_VECTOR \ > | OPTION_MASK_P9_VECTOR) > > -/* Support for a future processor's features. Do not enable -mpcrel until it > - is fully functional. */ > +/* Support for a future processor's features. Do not set > OPTION_MASK_PREFIXED > + or OPTION_MASK_PCREL here. Those options are enabled in the function > + rs6000_option_override if the ABI supports them. */ I am still not happy with that, and it should be fixed. But, let's do that later then. The ISA mask should say what features are supported. If we choose not to use some we should do that elsewhere, in option_override for example. Expressing only 95% of the features in the ISA masks is the worst of both worlds. > +/* Flags that need to be turned off if -mno-future. */ > +#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \ > | OPTION_MASK_PREFIXED) > > /* Flags that need to be turned off if -mno-future. */ And right after this is already a define for OTHER_FUTURE_MASKS. Did you mismerge this? Please correct and repost. > + /* Enable -mprefixed by default on 64-bit 'future' systems. */ > + if (TARGET_FUTURE && TARGET_POWERPC64 > + && (rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED) == 0) > + rs6000_isa_flags |= OPTION_MASK_PREFIXED; I don't understand why only for 64 bit? > +#ifdef PCREL_SUPPORTED_BY_OS > + /* If the ABI has support for PC-relative relocations, enable it by > + default. */ > + if (PCREL_SUPPORTED_BY_OS > + && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0) > + rs6000_isa_flags |= OPTION_MASK_PCREL; > +#endif No #ifdef at uses please, unless there is a reason (there is none here afaics). The #ifndef you had before for the default definition was fine. The #undef you had for a non-default definition was not: it hides problems, it never helps. Segher
