On Tue, Jul 15, 2025 at 08:11:05AM -0500, Segher Boessenkool wrote:
> Hi!
>
> On Tue, Jul 01, 2025 at 12:14:32PM -0400, Michael Meissner wrote:
> > This patch adds the support that can be used in developing GCC support
> > for potential future PowerPC processors.
> >
> > These changes are being added so that hardware designers can evaluate
> > potential new features to be added to the PowerPC processors in the
> > future. It may be these features will be incorporated into real
> > hardware using a different name in the future. Or it may be these
> > features will not be incoporated into actual PowerPC hardware in the
> > future.
>
> None of this belongs in the commit message. In the furure please post
> stuff exactly as you want it to be committed!
The patch includes comments regarding previous patches, and aren't the
commit patch. If I eliminate those comments, you will then say things
like 'where did this come from'.
I can try to separate things so there is the initial commentary meant
for the review to address concerns from previous patches.
> > * config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): If
> > -mcpu=future, define _ARCH_FUTURE.
> > * config/rs6000/rs6000-cpus.def (FUTURE_MASKS_SERVER): New macro.
> > (POWERPC_MASKS): Add OPTION_MASK_FUTURE.
> > (future cpu): Add 'future' cpu.
> > * config/rs6000/rs6000-tables.opt: Regenerate.
> > * config/rs6000/rs6000.opt (-mfuture-internal): New internal option to
> > indicate the user used -mcpu=future.
>
> NAK, as said before.
>
> If the user wants to see what rs6000_cpu is set to, the user should just
> look at rs6000_cpu, don't go via more indirect roundabout ways.
>
> > +/* At present, we are not providing a unique processor option for
> > -mcpu=future.
>
> What does this comment mean even?
We have essentially two choices, and I feel you have not liked either.
I don't see a clear way forward.
The way I did it before was to have a separate processor enumeration
for cpu=future. This involves adding switch statements and such for
every place we test for cpu == power10 or power11. And it also means
that power10.md has lots of edits to add the future case.
Or we can do it the way I did in this patch, to not create a new cpu
enumeration at the present time, and just set it to power11. Some day
in the future, sure we will move things around, but at the current
time, we are not adding unique scheduling for cpu=future.
It really doesn't matter to me, as I've done both approaches. Do you
want me to go back to having a separate future cpu enumeration, which
involves all of the changes we've seen in the past, or did you want the
more minimal changes that I just did.
> > + Until we define these changes, just use the power11 defaults. */
> > +RS6000_CPU ("future", PROCESSOR_POWER11, MASK_POWERPC64 |
> > FUTURE_MASKS_SERVER)
>
> It isn't talking about this code for sure.
>
> > +;; Users should not use -mfuture-internal, but we need to use a bit to
> > identify
> > +;; when the user changes the default cpu via #pragma GCC
> > target("cpu=future")
> > +;; and then resets it later.
>
> What does that mean? It hints at a bug elsewhere, but not more than
> hints unfortunately.
>
>
> Segher
--
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meiss...@linux.ibm.com
