diff mbox

[RFC] POWER8 default for PPC64LE

Message ID CAGWvnyk5WC96w5RbOq-D_2Pytur+bhqoHA5ZpRBe5Q5iwyiStg@mail.gmail.com
State New
Headers show

Commit Message

David Edelsohn Jan. 14, 2015, 8:32 p.m. UTC 
The PPC64LE ABI specifies POWER8 ISA as the minimum hardware
requierment.  Currently, Linux distributions are building the
toolchain using --with-cpu=power7 or power8, as they wish.  GCC
defaults to essentially the POWER4 ISA.

The appended patch changes the default for PPC64LE to POWER8 (ISA
2.7).  32-bit PPC SVR4 is not really defined, but it is left unchanged
with no minimum ISA.

The default ISA can be overridden using --with-cpu= and we presume
that Linux distributions and users will continue to configure as they
require for their deployment.

* config/rs6000/default64.h (TARGET_DEFAULT) [LITTLE_ENDIAN]: Use ISA
2.7 (POWER8).

Thanks, David

 #undef TARGET_DEFAULT
 #define TARGET_DEFAULT (MASK_PPC_GFXOPT | MASK_POWERPC64 | MASK_64BIT)

Comments

Jeff Law Jan. 14, 2015, 8:36 p.m. UTC | #1 
On 01/14/15 13:32, David Edelsohn wrote:
> The PPC64LE ABI specifies POWER8 ISA as the minimum hardware
> requierment.  Currently, Linux distributions are building the
> toolchain using --with-cpu=power7 or power8, as they wish.  GCC
> defaults to essentially the POWER4 ISA.
>
> The appended patch changes the default for PPC64LE to POWER8 (ISA
> 2.7).  32-bit PPC SVR4 is not really defined, but it is left unchanged
> with no minimum ISA.
>
> The default ISA can be overridden using --with-cpu= and we presume
> that Linux distributions and users will continue to configure as they
> require for their deployment.
>
> * config/rs6000/default64.h (TARGET_DEFAULT) [LITTLE_ENDIAN]: Use ISA
> 2.7 (POWER8).
Given you've got a new ABI in play here, seems like the perfect time to 
bump the default ISA to something reasonable.

jeff
Peter Bergner Jan. 15, 2015, 3:40 p.m. UTC | #2 
On Wed, 2015-01-14 at 13:36 -0700, Jeff Law wrote:
> On 01/14/15 13:32, David Edelsohn wrote:
> > The PPC64LE ABI specifies POWER8 ISA as the minimum hardware
> > requierment.  Currently, Linux distributions are building the
> > toolchain using --with-cpu=power7 or power8, as they wish.  GCC
> > defaults to essentially the POWER4 ISA.
> >
> > The appended patch changes the default for PPC64LE to POWER8 (ISA
> > 2.7).  32-bit PPC SVR4 is not really defined, but it is left unchanged
> > with no minimum ISA.
> >
> > The default ISA can be overridden using --with-cpu= and we presume
> > that Linux distributions and users will continue to configure as they
> > require for their deployment.
> >
> > * config/rs6000/default64.h (TARGET_DEFAULT) [LITTLE_ENDIAN]: Use ISA
> > 2.7 (POWER8).
> Given you've got a new ABI in play here, seems like the perfect time to 
> bump the default ISA to something reasonable.

Agreed.  Given the new ABI requires POWER8 as the minimum, I think this
patch is a no brainer.

Peter
Bill Schmidt Jan. 16, 2015, 3:52 p.m. UTC | #3 
Hi David,

Something's not quite right here -- we've been having bootstrap failures
with BE and LE PPC64 Linux.  Maybe a missing include?

g++ -c   -g -DIN_GCC    -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -
W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-
attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wn
o-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. -I. -I/home/gccbuild/gcc_t
runk_anonsvn/gcc/gcc -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/. -I/home/gccbui
ld/gcc_trunk_anonsvn/gcc/gcc/../include -I/home/gccbuild/gcc_trunk_anonsvn/gcc/g
cc/../libcpp/include  -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/../libdecnumber
 -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/../libdecnumber/dpd -I../libdecnumbe
r -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/../libbacktrace   -o options.o -MT 
options.o -MMD -MP -MF ./.deps/options.TPo options.c
In file included from tm.h:30:0,
                 from options.c:7:
/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/config/rs6000/default64.h:23:25: error:
 ‘ISA_2_7_MASKS_SERVER’ was not declared in this scope
 #define TARGET_DEFAULT (ISA_2_7_MASKS_SERVER | MASK_POWERPC64 | MASK_64BIT | MA
SK_LITTLE_ENDIAN)
                         ^
options.c:828:3: note: in expansion of macro ‘TARGET_DEFAULT’
   TARGET_DEFAULT, /* rs6000_isa_flags */
   ^
make[3]: *** [options.o] Error 1
make[3]: *** Waiting for unfinished jobs....

Bill



On Wed, 2015-01-14 at 15:32 -0500, David Edelsohn wrote:
> The PPC64LE ABI specifies POWER8 ISA as the minimum hardware
> requierment.  Currently, Linux distributions are building the
> toolchain using --with-cpu=power7 or power8, as they wish.  GCC
> defaults to essentially the POWER4 ISA.
> 
> The appended patch changes the default for PPC64LE to POWER8 (ISA
> 2.7).  32-bit PPC SVR4 is not really defined, but it is left unchanged
> with no minimum ISA.
> 
> The default ISA can be overridden using --with-cpu= and we presume
> that Linux distributions and users will continue to configure as they
> require for their deployment.
> 
> * config/rs6000/default64.h (TARGET_DEFAULT) [LITTLE_ENDIAN]: Use ISA
> 2.7 (POWER8).
> 
> Thanks, David
> 
> Index: default64.h
> ===================================================================
> --- default64.h (revision 219607)
> +++ default64.h (working copy)
> @@ -20,7 +20,7 @@
> 
>  #if (TARGET_DEFAULT & MASK_LITTLE_ENDIAN)
>  #undef TARGET_DEFAULT
> -#define TARGET_DEFAULT (MASK_PPC_GFXOPT | MASK_POWERPC64 | MASK_64BIT
> | MASK_LITTLE_ENDIAN)
> +#define TARGET_DEFAULT (ISA_2_7_MASKS_SERVER | MASK_POWERPC64 |
> MASK_64BIT | MASK_LITTLE_ENDIAN)
>  #else
>  #undef TARGET_DEFAULT
>  #define TARGET_DEFAULT (MASK_PPC_GFXOPT | MASK_POWERPC64 | MASK_64BIT)
>
Bill Schmidt Jan. 16, 2015, 4:04 p.m. UTC | #4 
On Fri, 2015-01-16 at 09:52 -0600, Bill Schmidt wrote:
> Hi David,
> 
> Something's not quite right here -- we've been having bootstrap failures
> with BE and LE PPC64 Linux.  Maybe a missing include?

Correction, just on LE.  The issue on BE was a different failure.

> 
> g++ -c   -g -DIN_GCC    -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -
> W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-
> attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wn
> o-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. -I. -I/home/gccbuild/gcc_t
> runk_anonsvn/gcc/gcc -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/. -I/home/gccbui
> ld/gcc_trunk_anonsvn/gcc/gcc/../include -I/home/gccbuild/gcc_trunk_anonsvn/gcc/g
> cc/../libcpp/include  -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/../libdecnumber
>  -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/../libdecnumber/dpd -I../libdecnumbe
> r -I/home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/../libbacktrace   -o options.o -MT 
> options.o -MMD -MP -MF ./.deps/options.TPo options.c
> In file included from tm.h:30:0,
>                  from options.c:7:
> /home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/config/rs6000/default64.h:23:25: error:
>  ‘ISA_2_7_MASKS_SERVER’ was not declared in this scope
>  #define TARGET_DEFAULT (ISA_2_7_MASKS_SERVER | MASK_POWERPC64 | MASK_64BIT | MA
> SK_LITTLE_ENDIAN)
>                          ^
> options.c:828:3: note: in expansion of macro ‘TARGET_DEFAULT’
>    TARGET_DEFAULT, /* rs6000_isa_flags */
>    ^
> make[3]: *** [options.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> 
> Bill
> 
> 
> 
> On Wed, 2015-01-14 at 15:32 -0500, David Edelsohn wrote:
> > The PPC64LE ABI specifies POWER8 ISA as the minimum hardware
> > requierment.  Currently, Linux distributions are building the
> > toolchain using --with-cpu=power7 or power8, as they wish.  GCC
> > defaults to essentially the POWER4 ISA.
> > 
> > The appended patch changes the default for PPC64LE to POWER8 (ISA
> > 2.7).  32-bit PPC SVR4 is not really defined, but it is left unchanged
> > with no minimum ISA.
> > 
> > The default ISA can be overridden using --with-cpu= and we presume
> > that Linux distributions and users will continue to configure as they
> > require for their deployment.
> > 
> > * config/rs6000/default64.h (TARGET_DEFAULT) [LITTLE_ENDIAN]: Use ISA
> > 2.7 (POWER8).
> > 
> > Thanks, David
> > 
> > Index: default64.h
> > ===================================================================
> > --- default64.h (revision 219607)
> > +++ default64.h (working copy)
> > @@ -20,7 +20,7 @@
> > 
> >  #if (TARGET_DEFAULT & MASK_LITTLE_ENDIAN)
> >  #undef TARGET_DEFAULT
> > -#define TARGET_DEFAULT (MASK_PPC_GFXOPT | MASK_POWERPC64 | MASK_64BIT
> > | MASK_LITTLE_ENDIAN)
> > +#define TARGET_DEFAULT (ISA_2_7_MASKS_SERVER | MASK_POWERPC64 |
> > MASK_64BIT | MASK_LITTLE_ENDIAN)
> >  #else
> >  #undef TARGET_DEFAULT
> >  #define TARGET_DEFAULT (MASK_PPC_GFXOPT | MASK_POWERPC64 | MASK_64BIT)
> > 
> 
>
David Edelsohn Jan. 18, 2015, 2:18 a.m. UTC | #5 
Supporting this turned out to be more involved.  --with-cpu implicitly
adds -mcpu to all specs, which invokes the assembler with the correct
option.  Directly setting TARGET_DEFAULT does not adjust the assembler
invocation.

This patch adds support to rs6000_file_start to emit the .machine
assembler pseudo-op to inform the assembler of the processor type.
For the moment, the .machine pseudo-op is not emitted if --with-cpu or
-mcpu= was used because the assembly explicitly is invoked with the
processor type.  The default is machine ppc64 or ppc.  The
implementation does not attempt to cover the various embedded
processors, but they already were configured explicitly.

The machine type only is emitted for ELF files.  Darwin already emits
the option and I am skipping this on AIX for the moment.

The patch also changes the default processor tuning to POWER8 for
PPC64 big endian Linux and detects POWER7 on AIX.

Thanks, David

        * config/rs6000/default64.h: Include rs6000-cpus.def.
        (TARGET_DEFAULT) [LITTLE_ENDIAN]: Use ISA 2.7 (POWER8).
        * config/rs6000/driver-rs6000.c (detect_processor_aix): Add POWER7.
        * config/rs6000/linux64.h: Always default to POWER8.
        * config/rs6000/rs6000.c (rs6000_file_start): Emit .machine
        pseudo-op to specify assembler dialect.
Alan Modra Jan. 18, 2015, 11:36 p.m. UTC | #6 
On Sat, Jan 17, 2015 at 09:18:14PM -0500, David Edelsohn wrote:
> Supporting this turned out to be more involved.  --with-cpu implicitly
> adds -mcpu to all specs, which invokes the assembler with the correct
> option.  Directly setting TARGET_DEFAULT does not adjust the assembler
> invocation.

Since you've been looking at this area, please consider reviewing
https://gcc.gnu.org/ml/gcc-patches/2014-01/msg01157.html
Andreas Schwab Jan. 21, 2015, 11:22 a.m. UTC | #7 
David Edelsohn <dje.gcc@gmail.com> writes:

> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 219747)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -5072,6 +5072,28 @@ rs6000_file_start (void)
>  	putc ('\n', file);
>      }
>  
> +#ifdef USING_ELFOS_H
> +  if (rs6000_default_cpu == 0 || rs6000_default_cpu[0] == '\0'
> +      || !global_options_set.x_rs6000_cpu_index)
> +    {
> +      fputs ("\t.machine ", asm_out_file);
> +      if ((TARGET_DEFAULT & OPTION_MASK_DIRECT_MOVE) != 0)
> +	fputs ("power8\n", asm_out_file);
> +      else if ((TARGET_DEFAULT & OPTION_MASK_POPCNTD) != 0)
> +	fputs ("power7\n", asm_out_file);
> +      else if ((TARGET_DEFAULT & OPTION_MASK_CMPB) != 0)
> +	fputs ("power6\n", asm_out_file);
> +      else if ((TARGET_DEFAULT & OPTION_MASK_POPCNTB) != 0)
> +	fputs ("power5\n", asm_out_file);
> +      else if ((TARGET_DEFAULT & OPTION_MASK_MFCRF) != 0)
> +	fputs ("power4\n", asm_out_file);
> +      else if ((TARGET_DEFAULT & OPTION_MASK_POWERPC64) != 0)
> +	fputs ("ppc64\n", asm_out_file);
> +      else
> +	fputs ("ppc\n", asm_out_file);
> +    }
> +#endif
> +

This is wrong, it doesn't account for -m64 on a --with-cpu=default32
compiler.

/home/abuild/rpmbuild/BUILD/gcc-5.0.0-r219892/obj-powerpc64-suse-linux/./gcc/xgcc -B/home/abuild/rpmbuild/BUILD/gcc-5.0.0-r219892/obj-powerpc64-suse-linux/./gcc/ -B/usr/powerpc64-suse-linux/bin/ -B/usr/powerpc64-suse-linux/lib/ -isystem /usr/powerpc64-suse-linux/include -isystem /usr/powerpc64-suse-linux/sys-include    -fmessage-length=0 -grecord-gcc-switches -O2 -D_FORTIFY_SOURCE=2 -funwind-tables -fasynchronous-unwind-tables -g -U_FORTIFY_SOURCE -m64 -O2  -fmessage-length=0 -grecord-gcc-switches -O2 -D_FORTIFY_SOURCE=2 -funwind-tables -fasynchronous-unwind-tables -g -U_FORTIFY_SOURCE -DIN_GCC    -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-format -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include   -fPIC -mlong-double-128 -mno-minimal-toc -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector   -fPIC -mlong-double-128 -mno-minimal-toc -I. -I. -I../../.././gcc -I../../../../libgcc -I../../../../libgcc/. -I../../../../libgcc/../gcc -I../../../../libgcc/../include -I../../../../libgcc/../libdecnumber/dpd -I../../../../libgcc/../libdecnumber -DHAVE_CC_TLS  -o _addvsi3.o -MT _addvsi3.o -MD -MP -MF _addvsi3.dep -DL_addvsi3 -c ../../../../libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS
/tmp/ccaoVOii.s: Assembler messages:
/tmp/ccaoVOii.s:33: Error: junk at end of line: `1'
/tmp/ccaoVOii.s:55: Error: junk at end of line: `1'
/tmp/ccaoVOii.s:95: Error: junk at end of line: `1'
/tmp/ccaoVOii.s:116: Error: junk at end of line: `1'
Makefile:466: recipe for target '_addvsi3.o' failed
make[5]: *** [_addvsi3.o] Error 1

See
<https://build.opensuse.org/package/live_build_log/devel:gcc/gcc5/openSUSE_Factory_PPC/ppc>
for the full log file.

Andreas.
David Edelsohn Jan. 21, 2015, 7:53 p.m. UTC | #8 
On Wed, Jan 21, 2015 at 6:22 AM, Andreas Schwab <schwab@suse.de> wrote:
> David Edelsohn <dje.gcc@gmail.com> writes:
>
>> Index: gcc/config/rs6000/rs6000.c
>> ===================================================================
>> --- gcc/config/rs6000/rs6000.c        (revision 219747)
>> +++ gcc/config/rs6000/rs6000.c        (working copy)
>> @@ -5072,6 +5072,28 @@ rs6000_file_start (void)
>>       putc ('\n', file);
>>      }
>>
>> +#ifdef USING_ELFOS_H
>> +  if (rs6000_default_cpu == 0 || rs6000_default_cpu[0] == '\0'
>> +      || !global_options_set.x_rs6000_cpu_index)
>> +    {
>> +      fputs ("\t.machine ", asm_out_file);
>> +      if ((TARGET_DEFAULT & OPTION_MASK_DIRECT_MOVE) != 0)
>> +     fputs ("power8\n", asm_out_file);
>> +      else if ((TARGET_DEFAULT & OPTION_MASK_POPCNTD) != 0)
>> +     fputs ("power7\n", asm_out_file);
>> +      else if ((TARGET_DEFAULT & OPTION_MASK_CMPB) != 0)
>> +     fputs ("power6\n", asm_out_file);
>> +      else if ((TARGET_DEFAULT & OPTION_MASK_POPCNTB) != 0)
>> +     fputs ("power5\n", asm_out_file);
>> +      else if ((TARGET_DEFAULT & OPTION_MASK_MFCRF) != 0)
>> +     fputs ("power4\n", asm_out_file);
>> +      else if ((TARGET_DEFAULT & OPTION_MASK_POWERPC64) != 0)
>> +     fputs ("ppc64\n", asm_out_file);
>> +      else
>> +     fputs ("ppc\n", asm_out_file);
>> +    }
>> +#endif
>> +
>
> This is wrong, it doesn't account for -m64 on a --with-cpu=default32
> compiler.

--with-cpu=default32 does not define TARGET_CPU_DEFAULT and
rs6000_default_cpu, so rs6000_file_start does not skip the new block
of code.

-m64 implicitly sets -mpowerpc64.

Error: junk at end of line: `1'

often is a problem with MRCRF (POWER4), which should not be enabled by
-m64 and -mpower64.

I tried configuring and building with --with-cpu=default32 on gcc110
running Fedora 18. I did not encounter the failure in stage 1. I
encountered a failure later in stage 2 because some 32 bit libraries
are not installed (which confirms that it's building PPC32).

Looking at the OpenSUSE Factor log in more detail, I see that the
configuration uses

--with-cpu=default32 --with-cpu-64=power4

The second option is the problem.  rs6000_start_file() probably should
use rs6000_isa_flags and not TARGET_DEFAULT.

Testing.

- David
diff mbox

Patch

Index: default64.h
===================================================================
--- default64.h (revision 219607)
+++ default64.h (working copy)
@@ -20,7 +20,7 @@ 

 #if (TARGET_DEFAULT & MASK_LITTLE_ENDIAN)
 #undef TARGET_DEFAULT
-#define TARGET_DEFAULT (MASK_PPC_GFXOPT | MASK_POWERPC64 | MASK_64BIT
| MASK_LITTLE_ENDIAN)
+#define TARGET_DEFAULT (ISA_2_7_MASKS_SERVER | MASK_POWERPC64 |
MASK_64BIT | MASK_LITTLE_ENDIAN)
 #else