| Message ID | 20200403003659.GA26283@ibm-tinman.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | Enable -mpcrel on PowerPC -mcpu=future ELF v2 systems, V3 | expand |
On Thu, 2020-04-02 at 20:36 -0400, Michael Meissner via Gcc-patches wrote: > Enable -mpcrel on PowerPC -mcpu=future ELF v2 systems, V3 > Hi, > This patch changes the default for -mcpu=future to be -mpcrel (i.e. > use > PC-relative addressing) if the ABI allows PC-relative relocations and > the user > did not use either -mno-pcrel or -mno-prefixed. > > I have changed the spelling of the macro to PCREL_SUPPORTED_BY_ABI > (from > PCREL_SUPPORTED_BY_OS) since you pointed out it is more properly a > function of > the particular ABI, rather than just an OS choice. I have changed > the various > comments to make it clearer. > > I have done a bootstrap and a make check with and without the patch > and there > were no regressions by adding the patch on a little endian PowerPC > Linux > system. > > I also tested by hand that if I use: > > -mcpu=power9 > -mcpu=future -mno-prefixed > -mcpu=future -mno-pcrel > -mcpu=future -mabi=elfv1 (or) > -mcpu=future -mcmodel=large > > that PC-relative addressing is not enabled by default. Variants of > this patch > have been used since December, building power8/power9 code on big > endian > systems, and power8/power9/future on little endian systems. Can I > check this > into the master branch? > > 2020-04-02 Michael Meissner <meissner@linux.ibm.com> > > * config/rs6000/linux64.h (PCREL_SUPPORTED_BY_ABI): Enable > prefixed PC-relative addressing if the ABI supports it. > * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do > not > set OPTION_MASK_FUTURE here. Patch contents suggest this should be "Do not set OPTION_MASK_PREFIXED here." > * config/rs6000/rs6000.c (rs6000_option_override_internal): > Enable > OPTION_MASK_PREFIXED and OPTION_MASK_PREFIXED on -mcpu=future > by > default if the current ABI allows the options. It's so good we enable it twice. :-) One of those should be s/OPTION_MASK_PREFIXED/OPTION_MASK_PCREL/ > > --- /tmp/IPB30g_linux64.h 2020-04-02 15:23:19.977060411 -0500 > +++ gcc/config/rs6000/linux64.h 2020-04-02 15:23:01.016474023 > -0500 > @@ -640,3 +640,11 @@ extern int dot_symbols; > enabling the __float128 keyword. */ > #undef TARGET_FLOAT128_ENABLE_TYPE > #define TARGET_FLOAT128_ENABLE_TYPE 1 > + > +/* Enable using prefixed PC-relative addressing on the 'future' > machine if the > + ABI supports it. The ELF v2 ABI only supports PC-relative > relocations for > + the medium code model. */ > +#undef PCREL_SUPPORTED_BY_ABI A previous comment suggested the #undef there should not be there. (potential for hiding or introducing a bug). Thanks -Will
Hi! On Thu, Apr 02, 2020 at 08:36:59PM -0400, Michael Meissner wrote: > I have changed the spelling of the macro to PCREL_SUPPORTED_BY_ABI (from > PCREL_SUPPORTED_BY_OS) since you pointed out it is more properly a function of > the particular ABI, rather than just an OS choice. No, I did not, and that is a much worse name. Please change it back. The ABI does not determine *at all* whether PC-relative instructions can be used. Most ABIs do not use any PC-relative instructions themselves, of course, but that is a very different thing. Also please fix the problems Will pointed out (thanks!), and do post new versions with the commit message you intend to use, please. Segher
--- /tmp/IPB30g_linux64.h 2020-04-02 15:23:19.977060411 -0500 +++ gcc/config/rs6000/linux64.h 2020-04-02 15:23:01.016474023 -0500 @@ -640,3 +640,11 @@ extern int dot_symbols; enabling the __float128 keyword. */ #undef TARGET_FLOAT128_ENABLE_TYPE #define TARGET_FLOAT128_ENABLE_TYPE 1 + +/* Enable using prefixed PC-relative addressing on the 'future' machine if the + ABI supports it. The ELF v2 ABI only supports PC-relative relocations for + the medium code model. */ +#undef PCREL_SUPPORTED_BY_ABI +#define PCREL_SUPPORTED_BY_ABI (TARGET_FUTURE && TARGET_PREFIXED \ + && ELFv2_ABI_CHECK \ + && (TARGET_CMODEL == CMODEL_MEDIUM)) --- /tmp/EHqpAk_rs6000-cpus.def 2020-04-02 15:23:19.993064282 -0500 +++ gcc/config/rs6000/rs6000-cpus.def 2020-04-02 15:23:01.016474023 -0500 @@ -75,11 +75,11 @@ | 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. We do not set the addressing + options OPTION_MASK_PREFIXED or OPTION_MASK_PCREL here. Those options are + enabled in the function rs6000_option_override if the ABI supports them. */ #define ISA_FUTURE_MASKS_SERVER (ISA_3_0_MASKS_SERVER \ - | OPTION_MASK_FUTURE \ - | OPTION_MASK_PREFIXED) + | OPTION_MASK_FUTURE) /* Flags that need to be turned off if -mno-future. */ #define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \ --- /tmp/Zayhhc_rs6000.c 2020-04-02 15:23:20.009068153 -0500 +++ gcc/config/rs6000/rs6000.c 2020-04-02 15:23:01.020474991 -0500 @@ -4020,6 +4020,11 @@ rs6000_option_override_internal (bool gl rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW; } + /* 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; + /* -mprefixed (and hence -mpcrel) requires -mcpu=future. */ if (TARGET_PREFIXED && !TARGET_FUTURE) { @@ -4181,6 +4186,14 @@ rs6000_option_override_internal (bool gl rs6000_isa_flags &= ~OPTION_MASK_PCREL; } +#ifdef PCREL_SUPPORTED_BY_ABI + /* If the ABI has support for PC-relative relocations, enable it by + default. */ + if (PCREL_SUPPORTED_BY_ABI + && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0) + rs6000_isa_flags |= OPTION_MASK_PCREL; +#endif + if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET) rs6000_print_isa_options (stderr, 0, "after subtarget", rs6000_isa_flags);