[rs6000] Fix PR target/63177: Powerpc no-vfa-vect-depend-2.c and no-vfa-vect-depend-3.c failures

PR63177 shows a bug in how we determine which gas options we decide to pass to the
assembler.  Normally, we pass the -m<CPU> option to the assembler if we used the
-mcpu=<CPU> option.  However, if we don't compile with -mcpu=<CPU>, then we will
check some of the -m<vector option> options and pass an appropriate -m<CPU> option
to the assembler.  This is all fine and good except for when we compile with
-mpower9-vector -mcpu=power8.  The problem here is that POWER9 added new lxvx/stxvx
instructions which already existed in POWER8 as extended mnemonics of lxvd2x/stxvd2x
which are different instructions and behave differently in LE mode.  The "bug" is
that -mpower9-vector enables the generation of the POWER9 lxvx instruction, but the
-mcpu=power8 option causes us to use the -mpower8 assembler option so we get the
wrong instruction. :-(

The fix used here is to catch the special case when we use -mpower9-vector and
-mcpu=power8 together and then force ourselves to use the -mpower9 gas option.

This passed bootstrap and regtesting with no regressions and fixes a little over
150 testsuite failures.  Ok for trunk and the appropriate release branches once
it's baked on trunk for a while?

Peter

	PR target/63177
	* /config/rs6000/rs6000.h (ASM_CPU_SPEC): Add support for -mpower9.
	Don't handle -mcpu=power8 if -mpower9-vector is also used.

On Mon, Jun 04, 2018 at 08:57:24PM -0500, Peter Bergner wrote:
> PR63177 shows a bug in how we determine which gas options we decide to pass to the
> assembler.  Normally, we pass the -m<CPU> option to the assembler if we used the
> -mcpu=<CPU> option.  However, if we don't compile with -mcpu=<CPU>, then we will
> check some of the -m<vector option> options and pass an appropriate -m<CPU> option
> to the assembler.  This is all fine and good except for when we compile with
> -mpower9-vector -mcpu=power8.  The problem here is that POWER9 added new lxvx/stxvx
> instructions which already existed in POWER8 as extended mnemonics of lxvd2x/stxvd2x
> which are different instructions and behave differently in LE mode.  The "bug" is
> that -mpower9-vector enables the generation of the POWER9 lxvx instruction, but the
> -mcpu=power8 option causes us to use the -mpower8 assembler option so we get the
> wrong instruction. :-(
> 
> The fix used here is to catch the special case when we use -mpower9-vector and
> -mcpu=power8 together and then force ourselves to use the -mpower9 gas option.

Ideally -mpowerN-vector will just go away.

> --- gcc/config/rs6000/rs6000.h	(revision 260913)
> +++ gcc/config/rs6000/rs6000.h	(working copy)
> @@ -120,7 +120,7 @@
>  %{mcpu=power6: %(asm_cpu_power6) -maltivec} \
>  %{mcpu=power6x: %(asm_cpu_power6) -maltivec} \
>  %{mcpu=power7: %(asm_cpu_power7)} \
> -%{mcpu=power8: %(asm_cpu_power8)} \
> +%{mcpu=power8: %{!mpower9-vector: %(asm_cpu_power8)}} \
>  %{mcpu=power9: %(asm_cpu_power9)} \
>  %{mcpu=a2: -ma2} \
>  %{mcpu=powerpc: -mppc} \
> @@ -169,6 +169,7 @@
>  %{maltivec: -maltivec} \
>  %{mvsx: -mvsx %{!maltivec: -maltivec} %{!mcpu*: %(asm_cpu_power7)}} \
>  %{mpower8-vector|mcrypto|mdirect-move|mhtm: %{!mcpu*: %(asm_cpu_power8)}} \
> +%{mpower9-vector: %{!mcpu*|mcpu=power8: %(asm_cpu_power9)}} \
>  -many"

Why do you need the !mpower9-vector in the mcpu=power8 clause?  Is how
mpower8-vector is handled not correct, or is something fundamentally
different there?


Segher

On 6/5/18 11:23 AM, Segher Boessenkool wrote:
> On Mon, Jun 04, 2018 at 08:57:24PM -0500, Peter Bergner wrote:
>> The fix used here is to catch the special case when we use -mpower9-vector and
>> -mcpu=power8 together and then force ourselves to use the -mpower9 gas option.
> 
> Ideally -mpowerN-vector will just go away.

No argument from me.



>> -%{mcpu=power8: %(asm_cpu_power8)} \
>> +%{mcpu=power8: %{!mpower9-vector: %(asm_cpu_power8)}} \
>>  %{mcpu=power9: %(asm_cpu_power9)} \
>>  %{mcpu=a2: -ma2} \
>>  %{mcpu=powerpc: -mppc} \
>> @@ -169,6 +169,7 @@
>>  %{maltivec: -maltivec} \
>>  %{mvsx: -mvsx %{!maltivec: -maltivec} %{!mcpu*: %(asm_cpu_power7)}} \
>>  %{mpower8-vector|mcrypto|mdirect-move|mhtm: %{!mcpu*: %(asm_cpu_power8)}} \
>> +%{mpower9-vector: %{!mcpu*|mcpu=power8: %(asm_cpu_power9)}} \
>>  -many"
> 
> Why do you need the !mpower9-vector in the mcpu=power8 clause?  Is how
> mpower8-vector is handled not correct, or is something fundamentally
> different there?

It's fundamentally different because of the overlapping lxvx/stxvx mnemonics
between P8 and P9, which doesn't occur between any other ISA levels.
Similar to gcc handling of options, gas uses the last -m<CPU> option to
assemble with, so if one were to pass -mpower9 -mpower8 to the assembler
(which you would get if you compile with -mpower9-vector -mcpu=power8),
then we'd assemble for power8 and get the P8's lxvx extended mnemonic.
So I've done that to "fix" any ordering issues.

Initially, I was thinking that we should determine what -m<CPU> gas option
to use by looking at the rs6000_isa_flags value to compute given all of the
gcc -m* and -mcpu=* options, but unfortunately, the gas option is computed
in the gcc driver and we don't have access to rs6000_isa_flags.  I thought
the fix above was the least of the bad solutions. :-)

Peter

On Tue, Jun 05, 2018 at 12:26:04PM -0500, Peter Bergner wrote:
> >> -%{mcpu=power8: %(asm_cpu_power8)} \
> >> +%{mcpu=power8: %{!mpower9-vector: %(asm_cpu_power8)}} \
> >>  %{mcpu=power9: %(asm_cpu_power9)} \
> >>  %{mcpu=a2: -ma2} \
> >>  %{mcpu=powerpc: -mppc} \
> >> @@ -169,6 +169,7 @@
> >>  %{maltivec: -maltivec} \
> >>  %{mvsx: -mvsx %{!maltivec: -maltivec} %{!mcpu*: %(asm_cpu_power7)}} \
> >>  %{mpower8-vector|mcrypto|mdirect-move|mhtm: %{!mcpu*: %(asm_cpu_power8)}} \
> >> +%{mpower9-vector: %{!mcpu*|mcpu=power8: %(asm_cpu_power9)}} \
> >>  -many"
> > 
> > Why do you need the !mpower9-vector in the mcpu=power8 clause?  Is how
> > mpower8-vector is handled not correct, or is something fundamentally
> > different there?
> 
> It's fundamentally different because of the overlapping lxvx/stxvx mnemonics
> between P8 and P9, which doesn't occur between any other ISA levels.
> Similar to gcc handling of options, gas uses the last -m<CPU> option to
> assemble with, so if one were to pass -mpower9 -mpower8 to the assembler
> (which you would get if you compile with -mpower9-vector -mcpu=power8),
> then we'd assemble for power8 and get the P8's lxvx extended mnemonic.
> So I've done that to "fix" any ordering issues.

Ah, that's the crux.  Thanks.  Add a comment to the code please?

> Initially, I was thinking that we should determine what -m<CPU> gas option
> to use by looking at the rs6000_isa_flags value to compute given all of the
> gcc -m* and -mcpu=* options, but unfortunately, the gas option is computed
> in the gcc driver and we don't have access to rs6000_isa_flags.  I thought
> the fix above was the least of the bad solutions. :-)

It isn't nice, but it looks like it will work.

Okay for trunk.  Thanks,


Segher

On 6/5/18 3:22 PM, Segher Boessenkool wrote:
> Ah, that's the crux.  Thanks.  Add a comment to the code please?

Like so?  I tried placing it next to the mcpu=power8 clause, but it seems
you cannot place comments there, as it creates syntax errors during the build.

 /* 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
-   you make changes here, make them also there.  */
+   you make changes here, make them also there.  PR63177: Do not pass -mpower8
+   to the assembler if -mpower9-vector was also used.  */
 #define ASM_CPU_SPEC \
...


Peter

On Tue, Jun 05, 2018 at 10:03:46PM -0500, Peter Bergner wrote:
> On 6/5/18 3:22 PM, Segher Boessenkool wrote:
> > Ah, that's the crux.  Thanks.  Add a comment to the code please?
> 
> Like so?  I tried placing it next to the mcpu=power8 clause, but it seems
> you cannot place comments there, as it creates syntax errors during the build.
> 
>  /* 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
> -   you make changes here, make them also there.  */
> +   you make changes here, make them also there.  PR63177: Do not pass -mpower8
> +   to the assembler if -mpower9-vector was also used.  */
>  #define ASM_CPU_SPEC \
> ...

Yes that's great, thanks!  Backports are okay, too.


Segher

On 6/6/18 2:54 PM, Segher Boessenkool wrote:
> On Tue, Jun 05, 2018 at 10:03:46PM -0500, Peter Bergner wrote:
>> On 6/5/18 3:22 PM, Segher Boessenkool wrote:
>>> Ah, that's the crux.  Thanks.  Add a comment to the code please?
>>
>> Like so?  I tried placing it next to the mcpu=power8 clause, but it seems
>> you cannot place comments there, as it creates syntax errors during the build.
>>
>>  /* 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
>> -   you make changes here, make them also there.  */
>> +   you make changes here, make them also there.  PR63177: Do not pass -mpower8
>> +   to the assembler if -mpower9-vector was also used.  */
>>  #define ASM_CPU_SPEC \
>> ...
> 
> Yes that's great, thanks!  Backports are okay, too.

Looking through the release branches, I had to backport this all the way
back to GCC 6.  Testing on all the release branches were clean so I have
committed everywhere.  Thanks!

Peter