| Message ID | 20190606224216.GA4582@ibm-toto.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | Disable PowerPC pc-relative support until the code is checked in | expand |
Hi Mike, On Thu, Jun 06, 2019 at 06:42:16PM -0400, Michael Meissner wrote: > 2019-06-06 Michael Meissner <meissner@linux.ibm.com> > > * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Delete > enabling -mprefixed-addr and -mpcrel by default. Why disable prefixed-addr? > * config/rs6000/rs6000.c (rs6000_option_override_internal): Make > -mpcrel and -mprefixed-addr act like other swtiches (i.e. using Typo ("switches"). > -mpcrel automatically sets -mcpu=future and -mprefixed-addr, and Automatically setting -mcpu= is a bad thing. Instead, we should just error out if someone tries to use -mpcrel with a CPU (or ABI, etc.) that doesn't support it. Or, is there any special reason you want it? In the future, we will not have an -mprefixed-addr option (it will be always on for CPUs that support it), and I don't see any real reason to allow disabling pcrel either, but we'll see. Segher
On Thu, Jun 06, 2019 at 06:14:29PM -0500, Segher Boessenkool wrote: > Hi Mike, > > On Thu, Jun 06, 2019 at 06:42:16PM -0400, Michael Meissner wrote: > > 2019-06-06 Michael Meissner <meissner@linux.ibm.com> > > > > * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Delete > > enabling -mprefixed-addr and -mpcrel by default. > > Why disable prefixed-addr? Convenience, but I could leave it in, since we don't yet have any prefixed instruction support. > > * config/rs6000/rs6000.c (rs6000_option_override_internal): Make > > -mpcrel and -mprefixed-addr act like other swtiches (i.e. using > > Typo ("switches"). Thanks. > > -mpcrel automatically sets -mcpu=future and -mprefixed-addr, and > > Automatically setting -mcpu= is a bad thing. Instead, we should just > error out if someone tries to use -mpcrel with a CPU (or ABI, etc.) that > doesn't support it. Or, is there any special reason you want it? Well, I was trying to be consistant with the other things (-mpower9-vector automaically sets all of the other power9 options). If you feel we don't need the consistancy, I can remove that part of the patch. As I mentioned elsewhere, there is a real problem with options specified on the command line and pragma/attribute target (basically if you set -mpcrel on the command line, and then do '#pragma GCC target ("cpu=power9")', it will currently complain that -mfuture or -mcpu=future is not set. I wanted to do the minimum patch so other people could start using the target. > In the future, we will not have an -mprefixed-addr option (it will be > always on for CPUs that support it), and I don't see any real reason > to allow disabling pcrel either, but we'll see. Well I suspect for at least several months we will need the ability to turn off pc-relative support but allow the other future stuff.
On Thu, Jun 06, 2019 at 07:53:29PM -0400, Michael Meissner wrote: > On Thu, Jun 06, 2019 at 06:14:29PM -0500, Segher Boessenkool wrote: > > On Thu, Jun 06, 2019 at 06:42:16PM -0400, Michael Meissner wrote: > > > 2019-06-06 Michael Meissner <meissner@linux.ibm.com> > > > -mpcrel automatically sets -mcpu=future and -mprefixed-addr, and > > > > Automatically setting -mcpu= is a bad thing. Instead, we should just > > error out if someone tries to use -mpcrel with a CPU (or ABI, etc.) that > > doesn't support it. Or, is there any special reason you want it? > > Well, I was trying to be consistant with the other things (-mpower9-vector > automaically sets all of the other power9 options). Yes, and that is no end of pain. It's better than just enabling *some* of the p9 insns, sure. > As I mentioned elsewhere, there is a real problem with options specified on the > command line and pragma/attribute target (basically if you set -mpcrel on the > command line, and then do '#pragma GCC target ("cpu=power9")', it will > currently complain that -mfuture or -mcpu=future is not set. That, for example. > > In the future, we will not have an -mprefixed-addr option (it will be > > always on for CPUs that support it), and I don't see any real reason > > to allow disabling pcrel either, but we'll see. > > Well I suspect for at least several months we will need the ability to turn off > pc-relative support but allow the other future stuff. Oh certainly. But we should keep the eventual goal in mind, and structure things for *that*, where possible. Segher
Index: gcc/config/rs6000/rs6000-cpus.def =================================================================== --- gcc/config/rs6000/rs6000-cpus.def (revision 272004) +++ gcc/config/rs6000/rs6000-cpus.def (working copy) @@ -77,9 +77,7 @@ /* Support for a future processor's features. */ #define ISA_FUTURE_MASKS_SERVER (ISA_3_0_MASKS_SERVER \ - | OPTION_MASK_FUTURE \ - | OPTION_MASK_PCREL \ - | OPTION_MASK_PREFIXED_ADDR) + | OPTION_MASK_FUTURE) \ /* Flags that need to be turned off if -mno-future. */ #define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \ Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 272004) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -3896,8 +3896,15 @@ rs6000_option_override_internal (bool gl ignore_masks = rs6000_disable_incompatible_switches (); /* For the newer switches (vsx, dfp, etc.) set some of the older options, - unless the user explicitly used the -mno-<option> to disable the code. */ - if (TARGET_P9_VECTOR || TARGET_MODULO || TARGET_P9_MISC) + unless the user explicitly used the -mno-<option> to disable the code. At + present -mfuture does not enable prefixed address or pc-relative support, + so if those are specified, enable the necessary additional bits. */ + if (TARGET_PCREL) + rs6000_isa_flags |= ((ISA_FUTURE_MASKS_SERVER + | OPTION_MASK_PREFIXED_ADDR) & ~ignore_masks); + else if (TARGET_PREFIXED_ADDR) + rs6000_isa_flags |= (ISA_FUTURE_MASKS_SERVER & ~ignore_masks); + else if (TARGET_P9_VECTOR || TARGET_MODULO || TARGET_P9_MISC) rs6000_isa_flags |= (ISA_3_0_MASKS_SERVER & ~ignore_masks); else if (TARGET_P9_MINMAX) { @@ -4248,7 +4255,7 @@ rs6000_option_override_internal (bool gl /* -mpcrel requires prefixed load/store addressing. */ if (TARGET_PCREL && !TARGET_PREFIXED_ADDR) { - if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0) + if ((rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED_ADDR) != 0) error ("%qs requires %qs", "-mpcrel", "-mprefixed-addr"); rs6000_isa_flags &= ~OPTION_MASK_PCREL; @@ -4257,7 +4264,7 @@ rs6000_option_override_internal (bool gl /* -mprefixed-addr (and hence -mpcrel) requires -mcpu=future. */ if (TARGET_PREFIXED_ADDR && !TARGET_FUTURE) { - if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0) + if ((rs6000_isa_flags_explicit & OPTION_MASK_FUTURE) != 0) error ("%qs requires %qs", "-mprefixed-addr", "-mcpu=future"); rs6000_isa_flags &= ~(OPTION_MASK_PCREL | OPTION_MASK_PREFIXED_ADDR); Index: gcc/testsuite/gcc.target/powerpc/localentry-1.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/localentry-1.c (revision 272004) +++ gcc/testsuite/gcc.target/powerpc/localentry-1.c (working copy) @@ -1,10 +1,11 @@ /* { dg-do compile } */ -/* { dg-options "-mdejagnu-cpu=future -O2" } */ +/* { dg-options "-mdejagnu-cpu=future -O2 -mpcrel" } */ /* { dg-require-effective-target powerpc_elfv2 } */ /* { dg-require-effective-target powerpc_future_ok } */ -/* Ensure we generate ".localentry fn,1" for both leaf and non-leaf - functions. */ +/* Ensure we generate ".localentry fn,1" for both leaf and non-leaf functions. + At present, -mcpu=future does not enable pc-relative mode, so make sure we + enable it to be able to check for .localentry. */ extern int y (int); Index: gcc/testsuite/gcc.target/powerpc/localentry-detect-1.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/localentry-detect-1.c (revision 272004) +++ gcc/testsuite/gcc.target/powerpc/localentry-detect-1.c (working copy) @@ -3,10 +3,12 @@ /* { dg-require-effective-target powerpc_future_ok } */ /* { dg-options "-O2 -mdejagnu-cpu=future" } */ - +/* At present, -mcpu=future does not enable pc-relative mode. Enable it here + explicitly until it is turned on by default. */ +#pragma GCC target ("cpu=future,pcrel") int localentry1 () { return 5; } -#pragma GCC target ("cpu=power9") +#pragma GCC target ("cpu=power9,no-pcrel") int localentry2 () { return 5; } /* { dg-final { scan-assembler {\.localentry\tlocalentry1,1\M} } } */ Index: gcc/testsuite/gcc.target/powerpc/notoc-direct-1.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/notoc-direct-1.c (revision 272004) +++ gcc/testsuite/gcc.target/powerpc/notoc-direct-1.c (working copy) @@ -1,10 +1,11 @@ /* { dg-do compile } */ -/* { dg-options "-mdejagnu-cpu=future -O2" } */ +/* { dg-options "-mdejagnu-cpu=future -O2 -mpcrel" } */ /* { dg-require-effective-target powerpc_elfv2 } */ /* { dg-require-effective-target powerpc_future_ok } */ -/* Test that calls generated from PC-relative code are - annotated with @notoc. */ +/* Test that calls generated from PC-relative code are annotated with @notoc. + At present, -mcpu=future does not enable pc-relative mode. Enable it here + explicitly until it is turned on by default. */ extern int yy0 (int); extern void yy1 (int); Index: gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c (revision 272004) +++ gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c (working copy) @@ -3,9 +3,12 @@ /* { dg-require-effective-target powerpc_elfv2 } */ /* { dg-require-effective-target powerpc_future_ok } */ -/* Test that potential sibcalls are not generated when the caller preserves - the TOC and the callee doesn't, or vice versa. */ +/* Test that potential sibcalls are not generated when the caller preserves the + TOC and the callee doesn't, or vice versa. At present, -mcpu=future does + not enable pc-relative mode. Enable it here explicitly until it is turned + on by default. */ +#pragma GCC target ("cpu=future,pcrel") int x (void) __attribute__((noinline)); int y (void) __attribute__((noinline)); int xx (void) __attribute__((noinline)); @@ -25,7 +28,7 @@ int sib_call (void) return x (); } -#pragma GCC target ("cpu=power9") +#pragma GCC target ("cpu=power9,no-pcrel") int normal_call (void) { return y (); @@ -36,7 +39,7 @@ int xx (void) return 1; } -#pragma GCC target ("cpu=future") +#pragma GCC target ("cpu=future,pcrel") int notoc_call (void) { return xx ();