Re: [PATCH] rs6000: Rework option -mpowerpc64 handling [PR106680]

[Segher Boessenkool <segher@kernel.crashing.org>]

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