| Message ID | 20181112114904.GH22752@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RS6000] Don't pass -many to the assembler | expand |
On Mon, Nov 12, 2018 at 10:19:04PM +1030, Alan Modra wrote: > I'd like to remove -many from the options passed by default to the > assembler, on the grounds that a gcc bug in instruction selection (eg. > emitting a power9 insn for -mcpu=power8) is better found at assembly > time than run time. > > This might annoy people for a while fixing user asm that we didn't > diagnose previously, but I believe this is the right direction to go. > Of course, -Wa,-many is available for anyone who just wants their > dodgy old code to work. > > Bootstrapped etc. powerpc64le-linux. OK? I forgot to mention something important. This exposes a bug with our target_clones support, in that we don't emit .machine directives when changing cpu. eg. gcc.target/powerpc/clone2.c fails with "unrecognized opcode: `modsd'". __attribute__((__target__("cpu=..."))) also doesn't emit a .machine directive before the affected function code.
Hi, On Mon, 12 Nov 2018, Alan Modra wrote: > I'd like to remove -many from the options passed by default to the > assembler, on the grounds that a gcc bug in instruction selection (eg. > emitting a power9 insn for -mcpu=power8) is better found at assembly > time than run time. > > This might annoy people for a while fixing user asm that we didn't > diagnose previously, but I believe this is the right direction to go. Of > course, -Wa,-many is available for anyone who just wants their dodgy old > code to work. Wouldn't this also break compiling code that contains power9 instructions but guarded by runtime tests to only be executed on power9 machines? That seems a valid usecase, and it'd be bad if the assembler fails to compile such. (You can't use -mcpu=power9 as work around as the other unguarded code is not supposed to be using power9 insns). Ciao, Michael. > > Bootstrapped etc. powerpc64le-linux. OK? > > * config/rs6000/rs6000.h (ASM_CPU_SPEC): Remove -many. > * config/rs6000/aix61.h (ASM_CPU_SPEC): Likewise. > * config/rs6000/aix71.h (ASM_CPU_SPEC): Likewise. > * testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c: Don't use > power mnemonics. > > diff --git a/gcc/config/rs6000/aix61.h b/gcc/config/rs6000/aix61.h > index 353e5d6cfeb..a7a8246bfe3 100644 > --- a/gcc/config/rs6000/aix61.h > +++ b/gcc/config/rs6000/aix61.h > @@ -91,8 +91,7 @@ do { \ > %{mcpu=630: -m620} \ > %{mcpu=970: -m970} \ > %{mcpu=G5: -m970} \ > -%{mvsx: %{!mcpu*: -mpwr6}} \ > --many" > +%{mvsx: %{!mcpu*: -mpwr6}}" > > #undef ASM_DEFAULT_SPEC > #define ASM_DEFAULT_SPEC "-mpwr4" > diff --git a/gcc/config/rs6000/aix71.h b/gcc/config/rs6000/aix71.h > index 2398ed64baa..d2ca8dc275d 100644 > --- a/gcc/config/rs6000/aix71.h > +++ b/gcc/config/rs6000/aix71.h > @@ -89,8 +89,7 @@ do { \ > maltivec: -m970; \ > maix64|mpowerpc64: -mppc64; \ > : %(asm_default)}; \ > - :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \ > --many" > + :%eMissing -mcpu option in ASM_SPEC_CPU?\n}" > > #undef ASM_DEFAULT_SPEC > #define ASM_DEFAULT_SPEC "-mpwr4" > diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h > index d75137cf8f5..9d78173a680 100644 > --- a/gcc/config/rs6000/rs6000.h > +++ b/gcc/config/rs6000/rs6000.h > @@ -137,8 +137,7 @@ > mvsx: -mpower7; \ > mpowerpc64: -mppc64;: %(asm_default)}; \ > :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \ > -%{mvsx: -mvsx -maltivec; maltivec: -maltivec} \ > --many" > +%{mvsx: -mvsx -maltivec; maltivec: -maltivec}" > > #define CPP_DEFAULT_SPEC "" > > diff --git a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c > index 14908dba690..eea7f6ffc2e 100644 > --- a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c > +++ b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c > @@ -45,14 +45,14 @@ __asm__ ("\t.globl\t" #NAME "_asm\n\t" \ > #NAME "_asm:\n\t" \ > "lis 11,gparms@ha\n\t" \ > "la 11,gparms@l(11)\n\t" \ > - "st 3,0(11)\n\t" \ > - "st 4,4(11)\n\t" \ > - "st 5,8(11)\n\t" \ > - "st 6,12(11)\n\t" \ > - "st 7,16(11)\n\t" \ > - "st 8,20(11)\n\t" \ > - "st 9,24(11)\n\t" \ > - "st 10,28(11)\n\t" \ > + "stw 3,0(11)\n\t" \ > + "stw 4,4(11)\n\t" \ > + "stw 5,8(11)\n\t" \ > + "stw 6,12(11)\n\t" \ > + "stw 7,16(11)\n\t" \ > + "stw 8,20(11)\n\t" \ > + "stw 9,24(11)\n\t" \ > + "stw 10,28(11)\n\t" \ > "stfd 1,32(11)\n\t" \ > "stfd 2,40(11)\n\t" \ > "stfd 3,48(11)\n\t" \ > >
On Nov 12 2018, Michael Matz <matz@suse.de> wrote: > Wouldn't this also break compiling code that contains power9 instructions > but guarded by runtime tests to only be executed on power9 machines? That > seems a valid usecase, and it'd be bad if the assembler fails to compile > such. (You can't use -mcpu=power9 as work around as the other > unguarded code is not supposed to be using power9 insns). You'll need to put .machine directives around them. Andreas.
On Mon, Nov 12, 2018 at 03:52:29PM +0100, Andreas Schwab wrote: > On Nov 12 2018, Michael Matz <matz@suse.de> wrote: > > > Wouldn't this also break compiling code that contains power9 instructions > > but guarded by runtime tests to only be executed on power9 machines? That > > seems a valid usecase, and it'd be bad if the assembler fails to compile > > such. (You can't use -mcpu=power9 as work around as the other > > unguarded code is not supposed to be using power9 insns). > > You'll need to put .machine directives around them. My worry with that is there may be too much legacy code that does not do this :-( Segher
Hi, On Mon, 12 Nov 2018, Segher Boessenkool wrote: > > > Wouldn't this also break compiling code that contains power9 > > > instructions but guarded by runtime tests to only be executed on > > > power9 machines? That seems a valid usecase, and it'd be bad if the > > > assembler fails to compile such. (You can't use -mcpu=power9 as > > > work around as the other unguarded code is not supposed to be using > > > power9 insns). > > > > You'll need to put .machine directives around them. > > My worry with that is there may be too much legacy code that does not do > this :-( We'll see once we put gcc9 through a distro build. My worry really only was that the change would result in compile breakage without a sensible solution. (I'll just give all packages whose build failures prevent gcc9 from being the new system compiler to Alan for fixing ;-) ). Ciao, Michael.
On 11/12/18 5:49 AM, Alan Modra wrote: > I'd like to remove -many from the options passed by default to the > assembler, on the grounds that a gcc bug in instruction selection (eg. > emitting a power9 insn for -mcpu=power8) is better found at assembly > time than run time. > > This might annoy people for a while fixing user asm that we didn't > diagnose previously, but I believe this is the right direction to go. > Of course, -Wa,-many is available for anyone who just wants their > dodgy old code to work. +1 Peter
On Mon, Nov 12, 2018 at 04:17:51PM +0000, Michael Matz wrote: > Hi, > > On Mon, 12 Nov 2018, Segher Boessenkool wrote: > > > > > Wouldn't this also break compiling code that contains power9 > > > > instructions but guarded by runtime tests to only be executed on > > > > power9 machines? That seems a valid usecase, and it'd be bad if the > > > > assembler fails to compile such. (You can't use -mcpu=power9 as > > > > work around as the other unguarded code is not supposed to be using > > > > power9 insns). > > > > > > You'll need to put .machine directives around them. > > > > My worry with that is there may be too much legacy code that does not do > > this :-( > > We'll see once we put gcc9 through a distro build. My worry really only > was that the change would result in compile breakage without a sensible > solution. (I'll just give all packages whose build failures prevent gcc9 > from being the new system compiler to Alan for fixing ;-) ). Heh. I've been using the patch (or one like it) myself for over 2 years, but of course I don't tend to compile whole distros. The length of time I've had it baking in my tree says something about my hesitation to post the patch more than anything else. Note that you can easily "fix" package build failures by adding -Wa,-many to CFLAGS. For people developing new code, it's the right way to go, and especially so for people working on gcc itself. For people just wanting stuff to compile, not so much. I fully expect a chorus of *MORON* or worse to come from the likes of the linux kernel rabble.
On Nov 12, 2018, at 3:13 PM, Alan Modra <amodra@gmail.com> wrote: > > For people developing new code, it's the right way to go, and > especially so for people working on gcc itself. For people just > wanting stuff to compile, not so much. I fully expect a chorus of > *MORON* or worse to come from the likes of the linux kernel rabble. So, if you just want to hear people whine... On darwin, we (darwin, as a platform decision) like all instructions available from the assembler. The assembler and the linker have specialized code to track all instructions used (from which CPU types those instructions come from), and mark the object file according to what is actually used. We also have FAT binaries as a standard feature and other things to make everything play nicely. People that use inline assembly are expected to know how to code, because it is an advanced feature, and not need hand holding on how to write the condition that guards the code. I don't recall seeing any reports of anyone needing any extra help in this matter. On darwin, there wasn't a .machine for a while, it came later. Anyway, I thought about saying that it would be nice if all platforms behaved the same, and ask, what do people thing the recommended behavior of all platforms should be? Personally I don't have a dog in this, as darwin cannot be changed, it's a platform feature, and personally, I don't write a ton of this type of code. I just provide an alternate POV. Darwin has api's to query the architecture and code in the assembler/linker to help manage it's decision. Normal ELF systems, I want to say, usually lack such things. So, choices it makes aren't necessarily right for others.
> On 13 Nov 2018, at 00:34, Mike Stump <mikestump@comcast.net> wrote: > > On Nov 12, 2018, at 3:13 PM, Alan Modra <amodra@gmail.com> wrote: >> >> For people developing new code, it's the right way to go, and >> especially so for people working on gcc itself. For people just >> wanting stuff to compile, not so much. I fully expect a chorus of >> *MORON* or worse to come from the likes of the linux kernel rabble. > > So, if you just want to hear people whine... > > On darwin, we (darwin, as a platform decision) like all instructions available from the assembler. The assembler and the linker have specialized code to track all instructions used (from which CPU types those instructions come from), and mark the object file according to what is actually used. We also have FAT binaries as a standard feature and other things to make everything play nicely. People that use inline assembly are expected to know how to code, because it is an advanced feature, and not need hand holding on how to write the condition that guards the code. I don't recall seeing any reports of anyone needing any extra help in this matter. On darwin, there wasn't a .machine for a while, it came later. > > Anyway, I thought about saying that it would be nice if all platforms behaved the same, and ask, what do people thing the recommended behavior of all platforms should be? > > Personally I don't have a dog in this, as darwin cannot be changed, it's a platform feature, and personally, I don't write a ton of this type of code. I just provide an alternate POV. Darwin has api's to query the architecture and code in the assembler/linker to help manage it's decision. Normal ELF systems, I want to say, usually lack such things. So, choices it makes aren't necessarily right for others. Given that we have our own assembler and platform equivalent of -many (-force_cpusubtype_ALL) .. I was just watching the thread go by ;) Having said that, it would be interesting to know what the recommendation is with .machine. Iain
On Mon, Nov 12, 2018 at 04:34:34PM -0800, Mike Stump wrote: > On Nov 12, 2018, at 3:13 PM, Alan Modra <amodra@gmail.com> wrote: > > > > For people developing new code, it's the right way to go, and > > especially so for people working on gcc itself. For people just > > wanting stuff to compile, not so much. I fully expect a chorus of > > *MORON* or worse to come from the likes of the linux kernel rabble. > > So, if you just want to hear people whine... I'm happy to hear other points of view. Ignore my hyperbole. > On darwin, we (darwin, as a platform decision) like all instructions available from the assembler. OK, fair enough. Another option is to just disable -many when gcc is in development, like we enable checking.
On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote: > On Mon, Nov 12, 2018 at 04:34:34PM -0800, Mike Stump wrote: > > On Nov 12, 2018, at 3:13 PM, Alan Modra <amodra@gmail.com> wrote: > > > > > > For people developing new code, it's the right way to go, and > > > especially so for people working on gcc itself. For people just > > > wanting stuff to compile, not so much. I fully expect a chorus of > > > *MORON* or worse to come from the likes of the linux kernel rabble. > > > > So, if you just want to hear people whine... > > I'm happy to hear other points of view. Ignore my hyperbole. > > > On darwin, we (darwin, as a platform decision) like all instructions available from the assembler. > > OK, fair enough. Another option is to just disable -many when gcc is > in development, like we enable checking. That is a good plan for GCC 9 at least. Segher
On 11/13/18 5:17 AM, Segher Boessenkool wrote: > On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote: >> On Mon, Nov 12, 2018 at 04:34:34PM -0800, Mike Stump wrote: >>> On Nov 12, 2018, at 3:13 PM, Alan Modra <amodra@gmail.com> wrote: >>> On darwin, we (darwin, as a platform decision) like all instructions available from the assembler. >> >> OK, fair enough. Another option is to just disable -many when gcc is >> in development, like we enable checking. > > That is a good plan for GCC 9 at least. I like the plan too. We can also continue to pass -many just for darwin if they really really think they need it. Peter
Hi Folks, > On 13 Nov 2018, at 17:48, Peter Bergner <bergner@linux.ibm.com> wrote: > > On 11/13/18 5:17 AM, Segher Boessenkool wrote: >> On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote: >>> On Mon, Nov 12, 2018 at 04:34:34PM -0800, Mike Stump wrote: >>>> On Nov 12, 2018, at 3:13 PM, Alan Modra <amodra@gmail.com> wrote: >>>> On darwin, we (darwin, as a platform decision) like all instructions available from the assembler. >>> >>> OK, fair enough. Another option is to just disable -many when gcc is >>> in development, like we enable checking. >> >> That is a good plan for GCC 9 at least. > > I like the plan too. We can also continue to pass -many just for darwin > if they really really think they need it. As far as I expect, Darwin should be untouched by this - we have a separate assembler (which doesn’t even respond to -many), so unless there’s some higher level translation done (it’s not mentioned in any Darwin specs), we should just carry on as before. When I do expect things to change is when multiple .machine directives are included in asm sources. (probably) the old cctools assembler won’t deal with them properly (the 4.0.1 era) LLVM-backend based version I have doesn’t deal with them either (this could be a general consideration for the other parts of the PPC toolchain). Having said that, I didn’t experiment with .machine on later LLVM backend versions yet. Thus, my current expectation is that this will be a NOP unless/until incompatible asm source changes are made. Iain
On 11/13/18 12:06 PM, Iain Sandoe wrote: > As far as I expect, Darwin should be untouched by this - we have a separate assembler (which doesn’t even respond to -many), so unless there’s some higher level translation done (it’s not mentioned in any Darwin specs), we should just carry on as before. Ah, good then. > When I do expect things to change is when multiple .machine directives are included in asm sources. > (probably) the old cctools assembler won’t deal with them properly Usually when there are multiple .machine's being used, they should be used with the ".machine push" and ".machine pop" directives so the temporary .machine value doesn't corrupt the .machine value being used for the rest of the file. Like so. .machine "power8" ... .machine push .machine "power9" <power9 insns> .machine pop ... Hopefully the cctools supports that. Peter
On Tue, Nov 13, 2018 at 05:17:41AM -0600, Segher Boessenkool wrote: > On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote: > > OK, fair enough. Another option is to just disable -many when gcc is > > in development, like we enable checking. > > That is a good plan for GCC 9 at least. Here's the patch. Bootstrapped etc. powerpc64le-linux with resultant fail of clone2 test as already noted. On top of https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00924.html so needs to be hand edited if applying without that patch. I'm going to be away for a few days without email access, which means I probably won't be seeing any replies until Monday. * config/rs6000/rs6000.h (ASM_OPT_ANY): Define. (ASM_CPU_SPEC): Conditionally add -many. * config/rs6000/aix61.h (ASM_CPU_SPEC): Likewise. * config/rs6000/aix71.h (ASM_CPU_SPEC): Likewise. * testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c: Don't use power mnemonics. diff --git a/gcc/config/rs6000/aix61.h b/gcc/config/rs6000/aix61.h index 353e5d6cfeb..809c5d8d599 100644 --- a/gcc/config/rs6000/aix61.h +++ b/gcc/config/rs6000/aix61.h @@ -91,8 +91,8 @@ do { \ %{mcpu=630: -m620} \ %{mcpu=970: -m970} \ %{mcpu=G5: -m970} \ -%{mvsx: %{!mcpu*: -mpwr6}} \ --many" +%{mvsx: %{!mcpu*: -mpwr6}}" \ +ASM_OPT_ANY #undef ASM_DEFAULT_SPEC #define ASM_DEFAULT_SPEC "-mpwr4" diff --git a/gcc/config/rs6000/aix71.h b/gcc/config/rs6000/aix71.h index 2398ed64baa..319bd2dc013 100644 --- a/gcc/config/rs6000/aix71.h +++ b/gcc/config/rs6000/aix71.h @@ -89,8 +89,8 @@ do { \ maltivec: -m970; \ maix64|mpowerpc64: -mppc64; \ : %(asm_default)}; \ - :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \ --many" + :%eMissing -mcpu option in ASM_SPEC_CPU?\n}" \ +ASM_OPT_ANY #undef ASM_DEFAULT_SPEC #define ASM_DEFAULT_SPEC "-mpwr4" diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index d75137cf8f5..613d16add69 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -72,6 +72,12 @@ #define PPC405_ERRATUM77 0 #endif +#if CHECKING_P +#define ASM_OPT_ANY "" +#else +#define ASM_OPT_ANY " -many" +#endif + /* Common ASM definitions used by ASM_SPEC among the various targets for handling -mcpu=xxx switches. There is a parallel list in driver-rs6000.c to provide the default assembler options if the user uses -mcpu=native, so if @@ -137,8 +143,8 @@ mvsx: -mpower7; \ mpowerpc64: -mppc64;: %(asm_default)}; \ :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \ -%{mvsx: -mvsx -maltivec; maltivec: -maltivec} \ --many" +%{mvsx: -mvsx -maltivec; maltivec: -maltivec}" \ +ASM_OPT_ANY #define CPP_DEFAULT_SPEC "" diff --git a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c index 14908dba690..eea7f6ffc2e 100644 --- a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c +++ b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c @@ -45,14 +45,14 @@ __asm__ ("\t.globl\t" #NAME "_asm\n\t" \ #NAME "_asm:\n\t" \ "lis 11,gparms@ha\n\t" \ "la 11,gparms@l(11)\n\t" \ - "st 3,0(11)\n\t" \ - "st 4,4(11)\n\t" \ - "st 5,8(11)\n\t" \ - "st 6,12(11)\n\t" \ - "st 7,16(11)\n\t" \ - "st 8,20(11)\n\t" \ - "st 9,24(11)\n\t" \ - "st 10,28(11)\n\t" \ + "stw 3,0(11)\n\t" \ + "stw 4,4(11)\n\t" \ + "stw 5,8(11)\n\t" \ + "stw 6,12(11)\n\t" \ + "stw 7,16(11)\n\t" \ + "stw 8,20(11)\n\t" \ + "stw 9,24(11)\n\t" \ + "stw 10,28(11)\n\t" \ "stfd 1,32(11)\n\t" \ "stfd 2,40(11)\n\t" \ "stfd 3,48(11)\n\t" \
On Nov 13, 2018, at 10:39 AM, Peter Bergner <bergner@linux.ibm.com> wrote: > > On 11/13/18 12:06 PM, Iain Sandoe wrote: >> As far as I expect, Darwin should be untouched by this - we have a separate assembler (which doesn’t even respond to -many), so unless there’s some higher level translation done (it’s not mentioned in any Darwin specs), we should just carry on as before. > > Ah, good then. > >> When I do expect things to change is when multiple .machine directives are included in asm sources. >> (probably) the old cctools assembler won’t deal with them properly > > Usually when there are multiple .machine's being used, they should be used > with the ".machine push" and ".machine pop" directives so the temporary > .machine value doesn't corrupt the .machine value being used for the rest > of the file. > Hopefully the cctools supports that. Likely not. The modern tools don't for x86_64.
On Wed, Nov 14, 2018 at 01:43:57PM +1030, Alan Modra wrote: > On Tue, Nov 13, 2018 at 05:17:41AM -0600, Segher Boessenkool wrote: > > On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote: > > > OK, fair enough. Another option is to just disable -many when gcc is > > > in development, like we enable checking. > > > > That is a good plan for GCC 9 at least. > > Here's the patch. Bootstrapped etc. powerpc64le-linux with resultant > fail of clone2 test as already noted. Revised again, with a bunch of related issues solved. Bootstrapped etc. powerpc64le-linux with no regressions. OK to apply mainline? --- I'd like to remove -many from the options passed by default to the assembler, on the grounds that a gcc bug in instruction selection (eg. emitting a power9 insn for -mcpu=power8) is better found at assembly time than run time. For now, just do this when --enable-checking or gcc is not a release. In contrast to the previous patch, I haven't changed any of the AIX header files in this patch. So AIX gcc will continue to pass -many to their assembler until someone else (David?) makes that change. This patch also emits .machine assembler directives for ELF targets when functions are compiled for different cpus via attributes or pragmas. That's necessary when the initial -m<cpu> option passed to the assembler doesn't enable the superset of all opcodes emitted, as seen by the earlier failure of gcc.target/powerpc/clone2.c (without .machine) when building gcc for power8. O3-pr70130.c also failed on an earlier version of this patch (when only testing one ISA bit to determine .machine). This is a test for a power7 vector bug, but on power8 hw check_vect_support_and_set_flags passes -mpower8-vector which means the test isn't exercising the original bug exactly. I reckon that is wrong, and similary for other vector testcases that ask for a specific cpu. I've fixed it here by explicitly passing -mno-power8-vector and similar vector options. * config/rs6000/rs6000.h (ASM_OPT_ANY): Define. (ASM_CPU_SPEC): Conditionally add -many. * config/rs6000/rs6000.c (rs6000_machine): New static var. (rs6000_machine_from_flags, emit_asm_machine): New functions.. (rs6000_file_start): ..extracted from here, and modified to test all ISA bits. (rs6000_output_function_prologue): Emit .machine as necessary. * testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c: Don't use power mnemonics. * testsuite/gcc.dg/vect/O3-pr70130.c: Disable default options added by check_vect_support_and_set_flags. * testsuite/gcc.dg/vect/pr48765.c: Likewise. * testsuite/gfortran.dg/vect/pr45714-b.f: Likewise. diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index f774e2d0bf7..4ca68d0a1d1 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -5715,6 +5715,36 @@ rs6000_builtin_md_vectorized_function (tree fndecl, tree type_out, /* Default CPU string for rs6000*_file_start functions. */ static const char *rs6000_default_cpu; +#ifdef USING_ELFOS_H +static const char *rs6000_machine; + +static const char * +rs6000_machine_from_flags (void) +{ + if ((rs6000_isa_flags & (ISA_3_0_MASKS_SERVER ^ ISA_2_7_MASKS_SERVER)) != 0) + return "power9"; + if ((rs6000_isa_flags & (ISA_2_7_MASKS_SERVER ^ ISA_2_6_MASKS_SERVER)) != 0) + return "power8"; + if ((rs6000_isa_flags & (ISA_2_6_MASKS_SERVER ^ ISA_2_5_MASKS_SERVER)) != 0) + return "power7"; + if ((rs6000_isa_flags & (ISA_2_5_MASKS_SERVER ^ ISA_2_4_MASKS)) != 0) + return "power6"; + if ((rs6000_isa_flags & (ISA_2_4_MASKS ^ ISA_2_1_MASKS)) != 0) + return "power5"; + if ((rs6000_isa_flags & ISA_2_1_MASKS) != 0) + return "power4"; + if ((rs6000_isa_flags & OPTION_MASK_POWERPC64) != 0) + return "ppc64"; + return "ppc"; +} + +static void +emit_asm_machine (void) +{ + fprintf (asm_out_file, "\t.machine %s\n", rs6000_machine); +} +#endif + /* Do anything needed at the start of the asm file. */ static void @@ -5780,27 +5810,10 @@ rs6000_file_start (void) } #ifdef USING_ELFOS_H + rs6000_machine = rs6000_machine_from_flags (); if (!(rs6000_default_cpu && rs6000_default_cpu[0]) && !global_options_set.x_rs6000_cpu_index) - { - fputs ("\t.machine ", asm_out_file); - if ((rs6000_isa_flags & OPTION_MASK_MODULO) != 0) - fputs ("power9\n", asm_out_file); - else if ((rs6000_isa_flags & OPTION_MASK_DIRECT_MOVE) != 0) - fputs ("power8\n", asm_out_file); - else if ((rs6000_isa_flags & OPTION_MASK_POPCNTD) != 0) - fputs ("power7\n", asm_out_file); - else if ((rs6000_isa_flags & OPTION_MASK_CMPB) != 0) - fputs ("power6\n", asm_out_file); - else if ((rs6000_isa_flags & OPTION_MASK_POPCNTB) != 0) - fputs ("power5\n", asm_out_file); - else if ((rs6000_isa_flags & OPTION_MASK_MFCRF) != 0) - fputs ("power4\n", asm_out_file); - else if ((rs6000_isa_flags & OPTION_MASK_POWERPC64) != 0) - fputs ("ppc64\n", asm_out_file); - else - fputs ("ppc\n", asm_out_file); - } + emit_asm_machine (); #endif if (DEFAULT_ABI == ABI_ELFv2) @@ -27757,7 +27770,17 @@ static void rs6000_output_function_prologue (FILE *file) { if (!cfun->is_thunk) - rs6000_output_savres_externs (file); + { + rs6000_output_savres_externs (file); +#ifdef USING_ELFOS_H + const char *curr_machine = rs6000_machine_from_flags (); + if (rs6000_machine != curr_machine) + { + rs6000_machine = curr_machine; + emit_asm_machine (); + } +#endif + } /* ELFv2 ABI r2 setup code and local entry point. This must follow immediately after the global entry point label. */ diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index e7e998d1492..2e2d253705b 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -70,6 +70,12 @@ #define PPC405_ERRATUM77 0 #endif +#if CHECKING_P +#define ASM_OPT_ANY "" +#else +#define ASM_OPT_ANY " -many" +#endif + /* Common ASM definitions used by ASM_SPEC among the various targets for handling -mcpu=xxx switches. There is a parallel list in driver-rs6000.c to provide the default assembler options if the user uses -mcpu=native, so if @@ -137,8 +143,8 @@ mvsx: -mpower7; \ mpowerpc64: -mppc64;: %(asm_default)}; \ :%eMissing -mcpu option in ASM_CPU_SPEC?\n} \ -%{mvsx: -mvsx -maltivec; maltivec: -maltivec} \ --many" +%{mvsx: -mvsx -maltivec; maltivec: -maltivec}" \ +ASM_OPT_ANY #define CPP_DEFAULT_SPEC "" diff --git a/gcc/testsuite/gcc.dg/vect/O3-pr70130.c b/gcc/testsuite/gcc.dg/vect/O3-pr70130.c index 18a295c83f0..f8b84405140 100644 --- a/gcc/testsuite/gcc.dg/vect/O3-pr70130.c +++ b/gcc/testsuite/gcc.dg/vect/O3-pr70130.c @@ -1,5 +1,5 @@ /* { dg-require-effective-target vsx_hw { target powerpc*-*-* } } */ -/* { dg-additional-options "-mcpu=power7" { target powerpc*-*-* } } */ +/* { dg-additional-options "-mcpu=power7 -mno-power9-vector -mno-power8-vector" { target powerpc*-*-* } } */ #include "tree-vect.h" diff --git a/gcc/testsuite/gcc.dg/vect/pr48765.c b/gcc/testsuite/gcc.dg/vect/pr48765.c index ae364379d07..b091a145d0f 100644 --- a/gcc/testsuite/gcc.dg/vect/pr48765.c +++ b/gcc/testsuite/gcc.dg/vect/pr48765.c @@ -1,6 +1,6 @@ /* { dg-do compile { target { powerpc*-*-* } } } */ /* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" } { "-mcpu=power6" } } */ -/* { dg-additional-options "-O3 -mcpu=power6" } */ +/* { dg-additional-options "-O3 -mcpu=power6 -mno-power9-vector -mno-power8-vector -mno-vsx" } */ enum reg_class { diff --git a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c index 14908dba690..eea7f6ffc2e 100644 --- a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c +++ b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c @@ -45,14 +45,14 @@ __asm__ ("\t.globl\t" #NAME "_asm\n\t" \ #NAME "_asm:\n\t" \ "lis 11,gparms@ha\n\t" \ "la 11,gparms@l(11)\n\t" \ - "st 3,0(11)\n\t" \ - "st 4,4(11)\n\t" \ - "st 5,8(11)\n\t" \ - "st 6,12(11)\n\t" \ - "st 7,16(11)\n\t" \ - "st 8,20(11)\n\t" \ - "st 9,24(11)\n\t" \ - "st 10,28(11)\n\t" \ + "stw 3,0(11)\n\t" \ + "stw 4,4(11)\n\t" \ + "stw 5,8(11)\n\t" \ + "stw 6,12(11)\n\t" \ + "stw 7,16(11)\n\t" \ + "stw 8,20(11)\n\t" \ + "stw 9,24(11)\n\t" \ + "stw 10,28(11)\n\t" \ "stfd 1,32(11)\n\t" \ "stfd 2,40(11)\n\t" \ "stfd 3,48(11)\n\t" \ diff --git a/gcc/testsuite/gfortran.dg/vect/pr45714-b.f b/gcc/testsuite/gfortran.dg/vect/pr45714-b.f index 0d00c6fd666..abf33cd25b8 100644 --- a/gcc/testsuite/gfortran.dg/vect/pr45714-b.f +++ b/gcc/testsuite/gfortran.dg/vect/pr45714-b.f @@ -1,5 +1,5 @@ ! { dg-do compile { target powerpc*-*-* } } -! { dg-additional-options "-O3 -mcpu=power7 -ffast-math -mveclibabi=mass" } +! { dg-additional-options "-O3 -mcpu=power7 -mno-power9-vector -mno-power8-vector -ffast-math -mveclibabi=mass" } integer index(18),i,j,k,l,ipiv(18),info,ichange,neq,lda,ldb, & nrhs,iplas
On Thu, Dec 13, 2018 at 5:26 AM Alan Modra <amodra@gmail.com> wrote: > > On Wed, Nov 14, 2018 at 01:43:57PM +1030, Alan Modra wrote: > > On Tue, Nov 13, 2018 at 05:17:41AM -0600, Segher Boessenkool wrote: > > > On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote: > > > > OK, fair enough. Another option is to just disable -many when gcc is > > > > in development, like we enable checking. > > > > > > That is a good plan for GCC 9 at least. > > > > Here's the patch. Bootstrapped etc. powerpc64le-linux with resultant > > fail of clone2 test as already noted. > > Revised again, with a bunch of related issues solved. Bootstrapped > etc. powerpc64le-linux with no regressions. OK to apply mainline? > > --- > I'd like to remove -many from the options passed by default to the > assembler, on the grounds that a gcc bug in instruction selection (eg. > emitting a power9 insn for -mcpu=power8) is better found at assembly > time than run time. > > For now, just do this when --enable-checking or gcc is not a release. > > In contrast to the previous patch, I haven't changed any of the AIX > header files in this patch. So AIX gcc will continue to pass -many to > their assembler until someone else (David?) makes that change. This > patch also emits .machine assembler directives for ELF targets when > functions are compiled for different cpus via attributes or pragmas. > That's necessary when the initial -m<cpu> option passed to the > assembler doesn't enable the superset of all opcodes emitted, as seen > by the earlier failure of gcc.target/powerpc/clone2.c (without > .machine) when building gcc for power8. The AIX release schedule and numbering of releases to support new processors is making it impossible to reliably predict which directives (processor names) will be supported on a system on which GCC is installed. It's safer to hide the problem on AIX with the use of -many. Thanks, David
diff --git a/gcc/config/rs6000/aix61.h b/gcc/config/rs6000/aix61.h index 353e5d6cfeb..a7a8246bfe3 100644 --- a/gcc/config/rs6000/aix61.h +++ b/gcc/config/rs6000/aix61.h @@ -91,8 +91,7 @@ do { \ %{mcpu=630: -m620} \ %{mcpu=970: -m970} \ %{mcpu=G5: -m970} \ -%{mvsx: %{!mcpu*: -mpwr6}} \ --many" +%{mvsx: %{!mcpu*: -mpwr6}}" #undef ASM_DEFAULT_SPEC #define ASM_DEFAULT_SPEC "-mpwr4" diff --git a/gcc/config/rs6000/aix71.h b/gcc/config/rs6000/aix71.h index 2398ed64baa..d2ca8dc275d 100644 --- a/gcc/config/rs6000/aix71.h +++ b/gcc/config/rs6000/aix71.h @@ -89,8 +89,7 @@ do { \ maltivec: -m970; \ maix64|mpowerpc64: -mppc64; \ : %(asm_default)}; \ - :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \ --many" + :%eMissing -mcpu option in ASM_SPEC_CPU?\n}" #undef ASM_DEFAULT_SPEC #define ASM_DEFAULT_SPEC "-mpwr4" diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index d75137cf8f5..9d78173a680 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -137,8 +137,7 @@ mvsx: -mpower7; \ mpowerpc64: -mppc64;: %(asm_default)}; \ :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \ -%{mvsx: -mvsx -maltivec; maltivec: -maltivec} \ --many" +%{mvsx: -mvsx -maltivec; maltivec: -maltivec}" #define CPP_DEFAULT_SPEC "" diff --git a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c index 14908dba690..eea7f6ffc2e 100644 --- a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c +++ b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c @@ -45,14 +45,14 @@ __asm__ ("\t.globl\t" #NAME "_asm\n\t" \ #NAME "_asm:\n\t" \ "lis 11,gparms@ha\n\t" \ "la 11,gparms@l(11)\n\t" \ - "st 3,0(11)\n\t" \ - "st 4,4(11)\n\t" \ - "st 5,8(11)\n\t" \ - "st 6,12(11)\n\t" \ - "st 7,16(11)\n\t" \ - "st 8,20(11)\n\t" \ - "st 9,24(11)\n\t" \ - "st 10,28(11)\n\t" \ + "stw 3,0(11)\n\t" \ + "stw 4,4(11)\n\t" \ + "stw 5,8(11)\n\t" \ + "stw 6,12(11)\n\t" \ + "stw 7,16(11)\n\t" \ + "stw 8,20(11)\n\t" \ + "stw 9,24(11)\n\t" \ + "stw 10,28(11)\n\t" \ "stfd 1,32(11)\n\t" \ "stfd 2,40(11)\n\t" \ "stfd 3,48(11)\n\t" \