Hi! On Mon, Mar 30, 2020 at 12:50:43PM -0500, will schmidt wrote: > On Fri, 2020-03-27 at 21:31 -0400, Michael Meissner via Gcc-patches > > * config/rs6000/rs6000.c (PCREL_SUPPORTED_BY_OS): New macro. > > (rs6000_option_override_internal): Set the -mprefixed and > > -mpcrel > > options for -mcpu=future if these options can be used. > > > s/can be used/are supported by the platform/ ?
The code says /* Enable -mprefixed by default on 64-bit 'future' systems. */ /* If the OS has support for PC-relative relocations, enable it now. */ and something like that should go in the changelog as well (two lines in changelog is fine -- they are two hunks of patch as well, anyway!) > > +/* Enable default support for PC-relative addressing on the 'future' > > system if > > + we can use the PC-relative instructions. Currently this support > > only exits > > exists > > > + for the ELF v2 object file format using the medium code > > model. */ > > should that be "s/object file format/ABI/" ? Yes. > > -/* Support for a future processor's features. Do not enable -mpcrel > > until it > > - is fully functional. */ > > +/* Support for a future processor's features. We do not set -mpcrel > > or > > + -mprefixed here. These bits are set in rs6000_option_override if > > the system > > + supports those options. */ > > I'm still not sure the comment here is actually necessary, there are > many other places where we also do not set -mpcrel or -mprefixed. If > history of the code here requires a hint to point at those options > being set in rs6000_option_override, then it's fine. If you really need to say you do *not* do something, you should say why not. Without that it only leaves more questions to the reader :-) Hopefully that then also explains why the reader should care about this. Segher
