| Message ID | 20201213001106.2C22833E81@hamza.pair.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Update the processor defaults for FreeBSD | expand |
On Sun, Dec 13, 2020 at 01:10:58AM +0100, Gerald Pfeifer wrote: > Piotr is the one spending most times on ensuring FreeBSD ports work > fine on POWER, so personally I'm happy to follow his recommendation > on such matters. I have a question though: > -/* Until now the 970 is the only Processor where FreeBSD 64-bit runs on. */ > #undef PROCESSOR_DEFAULT > -#define PROCESSOR_DEFAULT PROCESSOR_POWER4 > +#define PROCESSOR_DEFAULT PROCESSOR_PPC7450 > #undef PROCESSOR_DEFAULT64 > -#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4 > +#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 Is this wise? Binaries won't run on many BE systems anymore with this. (Linux still uses Power4 by default for BE, as well). In either case: yes, whatever you guys decide here is fine. Also for all backports you want. Thanks! Segher
Hello,
this is only default tuning (-mtune, not -mcpu). Linux also does similarly in linux64.h:
74 #undef PROCESSOR_DEFAULT
75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7
76 #undef PROCESSOR_DEFAULT64
77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
Although there is hard to explain (for me) setting to tune for POWER7 on 32-bits. POWER7 is 64-bit and it should be more reasonable to optimize for the last 32-bit machines that still may be in use (so G4).
That said, on the 64-bit front, we will just match the Linux setting.
On 20-12-13 08:23:53, Segher Boessenkool wrote:
> On Sun, Dec 13, 2020 at 01:10:58AM +0100, Gerald Pfeifer wrote:
> > Piotr is the one spending most times on ensuring FreeBSD ports work
> > fine on POWER, so personally I'm happy to follow his recommendation
> > on such matters.
>
> I have a question though:
>
> > -/* Until now the 970 is the only Processor where FreeBSD 64-bit runs on. */
> > #undef PROCESSOR_DEFAULT
> > -#define PROCESSOR_DEFAULT PROCESSOR_POWER4
> > +#define PROCESSOR_DEFAULT PROCESSOR_PPC7450
> > #undef PROCESSOR_DEFAULT64
> > -#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4
> > +#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
>
> Is this wise? Binaries won't run on many BE systems anymore with
> this. (Linux still uses Power4 by default for BE, as well).
>
> In either case: yes, whatever you guys decide here is fine. Also for
> all backports you want. Thanks!
>
>
> Segher
Hi! On Sun, Dec 13, 2020 at 03:34:57PM +0100, Piotr Kubaj wrote: > this is only default tuning (-mtune, not -mcpu). It is both, actually (-mcpu= implies -mtune=) > Linux also does similarly in linux64.h: > 74 #undef PROCESSOR_DEFAULT > 75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7 > 76 #undef PROCESSOR_DEFAULT64 > 77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 Yes, but we still default to power4 on BE Linux (and power8 on LE). I get lost in the macro maze here, not sure how that works :-) > Although there is hard to explain (for me) setting to tune for POWER7 on 32-bits. POWER7 is 64-bit and it should be more reasonable to optimize for the last 32-bit machines that still may be in use (so G4). 7450 has very different optimisation than 970. Power7 and 970 have largely similar pipelines. > That said, on the 64-bit front, we will just match the Linux setting. Compile a very simple test and look at the .machine in the generated .s? Does that do what you want? Segher
On 20-12-13 09:48:35, Segher Boessenkool wrote: > Hi! > > On Sun, Dec 13, 2020 at 03:34:57PM +0100, Piotr Kubaj wrote: > > this is only default tuning (-mtune, not -mcpu). > > It is both, actually (-mcpu= implies -mtune=) Yes, but -mtune doesn't imply -mcpu. If I set up only -mtune, -mcpu is the generic one (ppc970 for BE) > > > Linux also does similarly in linux64.h: > > 74 #undef PROCESSOR_DEFAULT > > 75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7 > > 76 #undef PROCESSOR_DEFAULT64 > > 77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 > > Yes, but we still default to power4 on BE Linux (and power8 on LE). I > get lost in the macro maze here, not sure how that works :-) > > > Although there is hard to explain (for me) setting to tune for POWER7 on 32-bits. POWER7 is 64-bit and it should be more reasonable to optimize for the last 32-bit machines that still may be in use (so G4). > > 7450 has very different optimisation than 970. Power7 and 970 have > largely similar pipelines. Yes, but the point is that users on 32-bit machines don't use POWER7, but most likely G4. > > > That said, on the 64-bit front, we will just match the Linux setting. > > Compile a very simple test and look at the .machine in the generated .s? > Does that do what you want? Yes, I just tested it (on BE). pkubaj@talos:$/usr/home/pkubaj$ cat test.c #include <stdio.h> int main() { printf("Hello world!"); } I built it twice, with GCC 9.3 without my patch and with GCC 10.1 with my patch. The only difference is: pkubaj@talos:$/usr/home/pkubaj$ diff -u test.s.9 test.s.10 --- test.s.9 2020-12-14 15:51:43.203263000 +0100 +++ test.s.10 2020-12-14 15:52:01.147238000 +0100 @@ -43,5 +43,5 @@ .cfi_endproc .LFE1: .size main,.-main - .ident "GCC: (FreeBSD Ports Collection) 9.3.0" + .ident "GCC: (FreeBSD Ports Collection) 10.2.0" .section .note.GNU-stack,"",@progbits Both have .machine power4: pkubaj@talos:$/usr/home/pkubaj$ grep .machine test.s.* test.s.10: .machine power4 test.s.9: .machine power4 I'm not sure where GCC sets power8 for LE, but it's definitely somewhere else. So IMO this patch is good to go. Thanks, Piotr Kubaj.
On Mon, Dec 14, 2020 at 03:57:27PM +0100, Piotr Kubaj wrote: > > It is both, actually (-mcpu= implies -mtune=) > Yes, but -mtune doesn't imply -mcpu. If I set up only -mtune, -mcpu is the generic one (ppc970 for BE) But that is not what the patch does? > > > Linux also does similarly in linux64.h: > > > 74 #undef PROCESSOR_DEFAULT > > > 75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7 > > > 76 #undef PROCESSOR_DEFAULT64 > > > 77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 > > > > Yes, but we still default to power4 on BE Linux (and power8 on LE). I > > get lost in the macro maze here, not sure how that works :-) I still am baffled how this works :-/ > > > Although there is hard to explain (for me) setting to tune for POWER7 on 32-bits. POWER7 is 64-bit and it should be more reasonable to optimize for the last 32-bit machines that still may be in use (so G4). > > > > 7450 has very different optimisation than 970. Power7 and 970 have > > largely similar pipelines. > Yes, but the point is that users on 32-bit machines don't use POWER7, but most likely G4. Ah, but most users using -m32 are on a 64-bit machine! > > > That said, on the 64-bit front, we will just match the Linux setting. > > > > Compile a very simple test and look at the .machine in the generated .s? > > Does that do what you want? > Yes, I just tested it (on BE). > pkubaj@talos:$/usr/home/pkubaj$ cat test.c > #include <stdio.h> > > int main() > { > printf("Hello world!"); > } > > I built it twice, with GCC 9.3 without my patch and with GCC 10.1 with my patch. The only difference is: > pkubaj@talos:$/usr/home/pkubaj$ diff -u test.s.9 test.s.10 > --- test.s.9 2020-12-14 15:51:43.203263000 +0100 > +++ test.s.10 2020-12-14 15:52:01.147238000 +0100 > @@ -43,5 +43,5 @@ > .cfi_endproc > .LFE1: > .size main,.-main > - .ident "GCC: (FreeBSD Ports Collection) 9.3.0" > + .ident "GCC: (FreeBSD Ports Collection) 10.2.0" > .section .note.GNU-stack,"",@progbits > > Both have .machine power4: > pkubaj@talos:$/usr/home/pkubaj$ grep .machine test.s.* > test.s.10: .machine power4 > test.s.9: .machine power4 > > I'm not sure where GCC sets power8 for LE, but it's definitely somewhere else. Yes... And I do not know where this power4 comes from. Mysteries :-) > So IMO this patch is good to go. Yes, thanks! Segher
On 20-12-14 10:22:32, Segher Boessenkool wrote: > On Mon, Dec 14, 2020 at 03:57:27PM +0100, Piotr Kubaj wrote: > > > It is both, actually (-mcpu= implies -mtune=) > > Yes, but -mtune doesn't imply -mcpu. If I set up only -mtune, -mcpu is the generic one (ppc970 for BE) > > But that is not what the patch does? OK, then can you explain to me what it does since the generated assembly still specifies .machine power4? This is my first attempt at improving GCC on powerpc* on FreeBSD and I'd really prefer not to break anything, but this SEEMS to be correct. > > > > > Linux also does similarly in linux64.h: > > > > 74 #undef PROCESSOR_DEFAULT > > > > 75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7 > > > > 76 #undef PROCESSOR_DEFAULT64 > > > > 77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 > > > > > > Yes, but we still default to power4 on BE Linux (and power8 on LE). I > > > get lost in the macro maze here, not sure how that works :-) > > I still am baffled how this works :-/ > > > > > Although there is hard to explain (for me) setting to tune for POWER7 on 32-bits. POWER7 is 64-bit and it should be more reasonable to optimize for the last 32-bit machines that still may be in use (so G4). > > > > > > 7450 has very different optimisation than 970. Power7 and 970 have > > > largely similar pipelines. > > Yes, but the point is that users on 32-bit machines don't use POWER7, but most likely G4. > > Ah, but most users using -m32 are on a 64-bit machine! > > > > > That said, on the 64-bit front, we will just match the Linux setting. > > > > > > Compile a very simple test and look at the .machine in the generated .s? > > > Does that do what you want? > > Yes, I just tested it (on BE). > > pkubaj@talos:$/usr/home/pkubaj$ cat test.c > > #include <stdio.h> > > > > int main() > > { > > printf("Hello world!"); > > } > > > > I built it twice, with GCC 9.3 without my patch and with GCC 10.1 with my patch. The only difference is: > > pkubaj@talos:$/usr/home/pkubaj$ diff -u test.s.9 test.s.10 > > --- test.s.9 2020-12-14 15:51:43.203263000 +0100 > > +++ test.s.10 2020-12-14 15:52:01.147238000 +0100 > > @@ -43,5 +43,5 @@ > > .cfi_endproc > > .LFE1: > > .size main,.-main > > - .ident "GCC: (FreeBSD Ports Collection) 9.3.0" > > + .ident "GCC: (FreeBSD Ports Collection) 10.2.0" > > .section .note.GNU-stack,"",@progbits > > > > Both have .machine power4: > > pkubaj@talos:$/usr/home/pkubaj$ grep .machine test.s.* > > test.s.10: .machine power4 > > test.s.9: .machine power4 > > > > I'm not sure where GCC sets power8 for LE, but it's definitely somewhere else. > > Yes... And I do not know where this power4 comes from. Mysteries :-) > > > So IMO this patch is good to go. > > Yes, thanks! > > > Segher --
On Mon, Dec 14, 2020 at 05:29:30PM +0100, Piotr Kubaj wrote: > On 20-12-14 10:22:32, Segher Boessenkool wrote: > > On Mon, Dec 14, 2020 at 03:57:27PM +0100, Piotr Kubaj wrote: > > > > It is both, actually (-mcpu= implies -mtune=) > > > Yes, but -mtune doesn't imply -mcpu. If I set up only -mtune, -mcpu is the generic one (ppc970 for BE) > > > > But that is not what the patch does? > OK, then can you explain to me what it does since the generated assembly still specifies .machine power4? It's a mystery to me! This macro *is* used for selecting the default -mcpu= setting. > This is my first attempt at improving GCC on powerpc* on FreeBSD and I'd really prefer not to break anything, but this SEEMS to be correct. You do everything the same as our Linux ports here, and all evidence shows it does in fact work, so it is fine. I just cannot see *why* or *how* it works :-) Segher
diff --git a/gcc/config/rs6000/freebsd64.h b/gcc/config/rs6000/freebsd64.h index 6984ca5a107..4b2da571c4a 100644 --- a/gcc/config/rs6000/freebsd64.h +++ b/gcc/config/rs6000/freebsd64.h @@ -51,11 +51,10 @@ extern int dot_symbols; #define SET_CMODEL(opt) do {} while (0) #endif -/* Until now the 970 is the only Processor where FreeBSD 64-bit runs on. */ #undef PROCESSOR_DEFAULT -#define PROCESSOR_DEFAULT PROCESSOR_POWER4 +#define PROCESSOR_DEFAULT PROCESSOR_PPC7450 #undef PROCESSOR_DEFAULT64 -#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4 +#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 /* We don't need to generate entries in .fixup, except when -mrelocatable or -mrelocatable-lib is given. */
Piotr is the one spending most times on ensuring FreeBSD ports work fine on POWER, so personally I'm happy to follow his recommendation on such matters. Okay for trunk and backports (GCC 10 at least)? Gerald gcc/ChangeLog: 2020-12-13 Piotr Kubaj <pkubaj@FreeBSD.org> Gerald Pfeifer <gerald@pfeifer.com> * config/rs6000/freebsd64.h (PROCESSOR_DEFAULT): Update to PROCESSOR_PPC7450. (PROCESSOR_DEFAULT64): Update to PROCESSOR_POWER8. --- gcc/config/rs6000/freebsd64.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)