On Thu, Oct 23, 2025 at 07:25:17PM +0530, Surya Kumari Jangala wrote:
> Hi Mike,
>
> On 23/09/25 3:32 am, Michael Meissner wrote:
> > See https://gcc.gnu.org/pipermail/gcc-patches/2025-September/695920.html for
> > (struct rs6000_cpu_opt_mask): Likewise.
>
> 'Likewise' is incorrect here. 'New type' perhaps?
Ok.
> > (rs6000_cpu_opt_masks): Likewise.
>
> 'New variable' instead of 'Likewise'.
Ok.
> > (rs6000_pragma_target_parse): Likewise.
> > (rs6000_function_specific_save): Likewise.
> > (rs6000_function_specific_restore): Likewise.
> > (rs6000_function_specific_print): Likewise.
> > (rs6000_print_options_internal): Likewise.
>
> Here too, 'Likewise' is incorrect.
Yes, I always struggle with the commit messages.
> > * config/rs6000/rs6000.opt (rs6000_cpu_option_flags): New target
> > variable.
> > (x_rs6000_cpu_option_flags): Likewise.
> > ---
> > gcc/config/rs6000/aix71.h | 2 +
> > gcc/config/rs6000/aix72.h | 2 +
> > gcc/config/rs6000/aix73.h | 2 +
> > gcc/config/rs6000/default64.h | 2 +
> > gcc/config/rs6000/driver-rs6000.cc | 2 +
> > gcc/config/rs6000/rs6000-c.cc | 12 ++-
> > gcc/config/rs6000/rs6000-cpus.def | 131 ++++++++++++++++++++++++-----
> > gcc/config/rs6000/rs6000-protos.h | 5 +-
> > gcc/config/rs6000/rs6000.cc | 129 +++++++++++++++++++++-------
> > gcc/config/rs6000/rs6000.opt | 9 ++
> > 10 files changed, 241 insertions(+), 55 deletions(-)
> >
> > diff --git a/gcc/config/rs6000/aix71.h b/gcc/config/rs6000/aix71.h
> > index 2b21dd7cd1e..5423252f768 100644
> > --- a/gcc/config/rs6000/aix71.h
> > +++ b/gcc/config/rs6000/aix71.h
> > @@ -133,8 +133,10 @@ do {
> > \
> > %{pthread: -D_THREAD_SAFE}"
> >
> > #define RS6000_CPU(NAME, CPU, FLAGS)
> > +#define RS6000_CPU_OPTION(NAME, CPU, FLAGS, CPU_OPTION)
>
> Instead of using CPU_OPTION, let's please make it as CPU_OPTION_FLAGS.
I've gone through several iterations of the names, but CPU_OPTION_FLAGS
is ok.
> > +/* CPU option mask bits that are set only via -mcpu=<xxx> options. */
> > +/* CPU option mask bits that are set only via -mcpu=<xxx> options. */
>
> Same line repeated twice.
Thanks.
> > @@ -2414,19 +2421,19 @@ rs6000_debug_reg_global (void)
> > = processor_target_table[rs6000_tune_index].target_enable;
> >
> > sprintf (flags_buffer, "-mtune=%s flags", name);
> > - rs6000_print_isa_options (stderr, 0, flags_buffer, flags);
> > + rs6000_print_isa_options (stderr, 0, flags_buffer, flags, 0);
>
> We should print cpu_option here too. Any reason why we are just printing 0?
For -mtune=<xxx>, the cpu options really don't matter, and the
-mcpu=<xxx> was printed previously.
> > }
> > else
> > fprintf (stderr, DEBUG_FMT_S, "tune", "<none>");
> >
> > cl_target_option_save (&cl_opts, &global_options, &global_options_set);
> > rs6000_print_isa_options (stderr, 0, "rs6000_isa_flags",
> > - rs6000_isa_flags);
> > + rs6000_isa_flags, rs6000_cpu_option_flags);
> >
> > rs6000_print_isa_options (stderr, 0, "rs6000_isa_flags_explicit",
> > - rs6000_isa_flags_explicit);
> > + rs6000_isa_flags_explicit, 0);
>
> If we have a new feature in the future power processor, say "foo", and we
> want to allow the user to enable/disable this feature in gcc, then how is
> "foo" represented? Will it be a part of 'target_enable'or will it be a
> part of 'cpu_option' in 'struct rs6000_ptt' ? If it will be a part of
> 'cu_option', then we should be printing them just like we are printing
> 'rs6000_isa_flags_explicit'.
This is one of a balance issue.
In terms of future stuff, after the -mcpu=future stuff, I want to
re-submit the patches to enable potential dense math registers and a
potential future subsystem to extend the MMA system.
I would imagine that we would add the general -mcpu=future just to
provide a platform to hang changes for various potential changes.
With this potential new feature, we would likely want the user to have
the ability to turn this on and off. So we would add a new ISA flag
(say -mdense-math-registers). The -mdense-math-registers flag would be
checked and not alowed unless the cpu is 'future' and -mmma is set.
The user can do -mno-dense-math-registers if they want to use the
current MMA system instead of MMA+. Within the GCC source, the code
would all use TARGET_DENSE_MATH_REGISTERS (or preferbly TARGET_DMR).
On the other hand, say we want to look at potential new variants of the
lxvl (load vector variable) and stxvl (store vector variable)
instructions that take the vector length in a more convenient location
in the vector registers called lxvrl and stxvrl (this more convenient
would eliminate doing a shift to get the byte count in the correct
location). With these potential new instructions, it is not worth it
add -mlxvrl and -mno-lxvrl options. Instead, we would add a macro like
the following in rs6000.h:
#define TARGET_LXVRL TARGET_FUTURE
and the code in rs6000-string.cc would use TARGET_LXVRL for the tests.
In the future, if we decide to add a power53 processor that provides
lxvrl, we would change the macro in rs6000.h to say:
#define TARGET_LXVRL TARGET_POWER53
Doing it this way means, we only need to change one line rather than
tracking down all of the TARGET_FUTURE references to change them to
TARGET_POWER53.
Now, these changes were meant to fix some long standing issues.
We could instead decide to go back to the current way of doing things,
and adding 2 ISA options:
-mfuture: Enable -mcpu=future support in general
-mdense-math-registers: Enable dense math register support.
We would need to accept that sooner or later, somebody probably will
say -mfuture instead of -mcpu=future.
Then we would use tests for TARGET_DENSE_MATH_REGISTERS or TARGET_DMR
for this potential new MMA+ support, and we would have TARGET_LXVRL
macro based on -mfuture, but maybe not add -mlxvrl and -mno-lxvrl
options.
It would be nice to clean up this mess, but we can kick the can down
the road and use the current method.
--
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: [email protected]
