On 5/22/19 6:05 PM, Segher Boessenkool wrote:
> Hi Bill,
>
> On Wed, May 22, 2019 at 12:51:50PM -0500, Bill Schmidt wrote:
>> +/* Define if your assembler supports FUTURE instructions. */
>> +#ifndef USED_FOR_TARGET
>> +#undef HAVE_AS_FUTURE
>> +#endif
> Let's not use that? I removed HAVE_AS_* (for ISA version support) in
> r264675:
Yep, agreed. This was refactored from old patches prior to that.
>
> """
> rs6000: Delete many HAVE_AS_* (PR87149)
>
> This deletes most HAVE_AS_* that determine if the assembler supports
> some ISA level (and also HAVE_AS_MFPGPR and HAVE_AS_DFP).
>
> These are not useful: we will only generate an instruction that requires
> some newer ISA if the user specifically asked for it (with -mcpu=, say).
> If the assembler cannot handle that, it is fine if it gives an error.
>
> They also hurt: it increases the number of possible situations that all
> need handling and all need testing. We do not handle all cases, and
> obviously do not test all either.
> """
>
> So, just generate FUTURE insns when the user asked for that, don't worry
> about checking if the assembler can handle that -- the user will find out
> soon enough if not, and on any normal install everything will work fine.
>
>> /* Flags that need to be turned off if -mno-power9-vector. */
>> #define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW
>> \
>> + | OPTION_MASK_FUTURE \
>> | OPTION_MASK_P9_MINMAX)
> Why this?
I agree that this seems wrong. Will remove.
>
>> @@ -134,7 +139,8 @@
>> | OPTION_MASK_RECIP_PRECISION \
>> | OPTION_MASK_SOFT_FLOAT \
>> | OPTION_MASK_STRICT_ALIGN_OPTIONAL \
>> - | OPTION_MASK_VSX)
>> + | OPTION_MASK_VSX \
>> + | OPTION_MASK_FUTURE)
> This list is alphabetic. Well, was alphabetic :-)
OK.
>
>> -RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 |
>> ISA_2_7_MASKS_SERVER)
>> +RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64
>> + | ISA_2_7_MASKS_SERVER)
> Please don't reformat mixed with other (non-random) changes.
OK.
>
>> -@samp{power9}, @samp{powerpc}, @samp{powerpc64}, @samp{powerpc64le},
>> -@samp{rs64}, and @samp{native}.
>> +@samp{power9}, @samp{future}, @samp{powerpc}, @samp{powerpc64},
>> +@samp{powerpc64le}, @samp{rs64}, and @samp{native}.
> Maybe *not* documenting this would be less confusing?
Well, this was refactored from your patch. ;-)
I disagree, simply because at some point this will be renamed, and
having the place-holder will ensure we don't forget to add the new
name. So I'd like to keep this.
>
>> diff --git a/gcc/testsuite/gcc.target/powerpc/cpu-future.c
>> b/gcc/testsuite/gcc.target/powerpc/cpu-future.c
>> new file mode 100644
>> index 00000000000..b62d40341d1
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/cpu-future.c
>> @@ -0,0 +1,7 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-mcpu=future -O2" } */
>> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
>> "-mcpu=future" } } */
> Drop this last line, use -mdejagnu-cpu=future instead?
OK.
Thanks for the quick review!
Bill
>
>
> Segher
>
