On Thu, Sep 29, 2022 at 01:45:16PM +0800, Kewen.Lin wrote:
> I found this flag is mainly related to tune setting and spotted that we have 
> some code
> for tune setting when no explicit cpu is given. 
> 
> ...
> 
>   else
>     {
>       size_t i;
>       enum processor_type tune_proc
>       = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
> 
>       tune_index = -1;
>       for (i = 0; i < ARRAY_SIZE (processor_target_table); i++)
>       if (processor_target_table[i].processor == tune_proc)
>         {
>           tune_index = i;
>           break;
>         }
>     }

Ah cool, that needs fixing yes.

> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -3702,7 +3702,7 @@ rs6000_option_override_internal (bool global_init_p)
>        else
>       {
>         /* PowerPC 64-bit LE requires at least ISA 2.07.  */
> -       const char *default_cpu = (!TARGET_POWERPC64
> +       const char *default_cpu = (!TARGET_POWERPC64 && TARGET_32BIT
>                                    ? "powerpc"
>                                    : (BYTES_BIG_ENDIAN
>                                       ? "powerpc64"

... but not like that.  If this snippet should happen later just move it
later.  Or introduce a new variable to make the control flow less
confused.  Or something else.  But don't make the code more complex,
introducing more special cases like this.

> +#ifdef OS_MISSING_POWERPC64
> +      else if (OS_MISSING_POWERPC64)
> +     /* It's unexpected to have OPTION_MASK_POWERPC64 on for OSes which
> +        miss powerpc64 support, so disable it.  */
> +     rs6000_isa_flags &= ~OPTION_MASK_POWERPC64;
> +#endif

All silent stuff is always bad.

If things are done well, we will end up with *less* code than what we
had before, not more!


Segher

Reply via email to