Re: Repost [PATCH 1/6] Add -mcpu=future

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

On Fri, Jan 05, 2024 at 06:35:37PM -0500, Michael Meissner wrote:
> 	* config/rs6000/rs6000.opt (-mfuture): New undocumented debug switch.

No.  Never ever use a flag that does what -mcpu=<smth> should do.  We're
still trying to recover from previous such mistakes.  Don't add more
please.

> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -447,6 +447,8 @@ rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR9");
>    if ((flags & OPTION_MASK_POWER10) != 0)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR10");
> +  if ((flags & OPTION_MASK_FUTURE) != 0)
> +    rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR_FUTURE");

if ((((a & B) != 0) != 0) != 0)  ?  You can do just
if (a & B)

Yes, existing code already does the silly thing, but just fix it then,
don't add more :-)

(And no   if ((a & B))   either please).

> +static int
> +rs600_cpu_index_lookup (enum processor_type processor)
> +{
> +  for (size_t i = 0; i < ARRAY_SIZE (processor_target_table); i++)
> +    if (processor_target_table[i].processor == processor)
> +      return i;
> +
> +  return -1;
> +}

"int i" please, not "size_t".  This has nothing to do with object sizes.
The loop counter will always be a small number.

> +  /* At the moment, we don't have explict -mtune=future support.  If the user

"At the moment" is out of date almost as soon as you write it.  It is
better to avoid such terms ;-)

> +     explicitly tried to use -mtune=future, give a warning.  If not, use the
> +     power10 tuning until future tuning is added.  */

There should be Power11 tuning now, please use that?

So please post this -- as a separate series, and not as a single patch --
after fixing the things Ke Wen pointed out.  Thanks!


Segher