Repost [PATCH 1/6] Add -mcpu=future
Checks
Commit Message
This patch implements support for a potential future PowerPC cpu. Features
added with -mcpu=future, may or may not be added to new PowerPC processors.
This patch adds support for the -mcpu=future option. If you use -mcpu=future,
the macro __ARCH_PWR_FUTURE__ is defined, and the assembler .machine directive
"future" is used. Future patches in this series will add support for new
instructions that may be present in future PowerPC processors.
This particular patch does not any new features. It exists as a ground work
for future patches to support for a possible PowerPC processor in the future.
This patch does not implement any differences in tuning when -mcpu=future is
used compared to -mcpu=power10. If -mcpu=future is used, GCC will use power10
tuning. If you explicitly use -mtune=future, you will get a warning that
-mtune=future is not supported, and default tuning will be set for power10.
The patches have been tested on both little and big endian systems. Can I check
it into the master branch?
2024-01-05 Michael Meissner <meissner@linux.ibm.com>
gcc/
* config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): Define
__ARCH_PWR_FUTURE__ if -mcpu=future.
* config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS): New macro.
(POWERPC_MASKS): Add -mcpu=future support.
* config/rs6000/rs6000-opts.h (enum processor_type): Add
PROCESSOR_FUTURE.
* config/rs6000/rs6000-tables.opt: Regenerate.
* config/rs6000/rs6000.cc (rs600_cpu_index_lookup): New helper
function.
(rs6000_option_override_internal): Make -mcpu=future set
-mtune=power10. If the user explicitly uses -mtune=future, give a
warning and reset the tuning to power10.
(rs6000_option_override_internal): Use power10 costs for future
machine.
(rs6000_machine_from_flags): Add support for -mcpu=future.
(rs6000_opt_masks): Likewise.
* config/rs6000/rs6000.h (ASM_CPU_SUPPORT): Likewise.
* config/rs6000/rs6000.md (cpu attribute): Likewise.
* config/rs6000/rs6000.opt (-mfuture): New undocumented debug switch.
* doc/invoke.texi (IBM RS/6000 and PowerPC Options): Document -mcpu=future.
---
gcc/config/rs6000/rs6000-c.cc | 2 +
gcc/config/rs6000/rs6000-cpus.def | 6 +++
gcc/config/rs6000/rs6000-opts.h | 4 +-
gcc/config/rs6000/rs6000-tables.opt | 3 ++
gcc/config/rs6000/rs6000.cc | 58 ++++++++++++++++++++++++-----
gcc/config/rs6000/rs6000.h | 1 +
gcc/config/rs6000/rs6000.md | 2 +-
gcc/config/rs6000/rs6000.opt | 4 ++
gcc/doc/invoke.texi | 2 +-
9 files changed, 69 insertions(+), 13 deletions(-)
Comments
Ping
| Date: Fri, 5 Jan 2024 18:35:37 -0500
| From: Michael Meissner <meissner@linux.ibm.com>
| Subject: Repost [PATCH 1/6] Add -mcpu=future
| Message-ID: <ZZiSSeBqMdd64W7V@cowardly-lion.the-meissners.org>
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/641961.html
Hi Mike,
on 2024/1/6 07:35, Michael Meissner wrote:
> This patch implements support for a potential future PowerPC cpu. Features
> added with -mcpu=future, may or may not be added to new PowerPC processors.
>
> This patch adds support for the -mcpu=future option. If you use -mcpu=future,
> the macro __ARCH_PWR_FUTURE__ is defined, and the assembler .machine directive
> "future" is used. Future patches in this series will add support for new
> instructions that may be present in future PowerPC processors.
>
> This particular patch does not any new features. It exists as a ground work
> for future patches to support for a possible PowerPC processor in the future.
>
> This patch does not implement any differences in tuning when -mcpu=future is
> used compared to -mcpu=power10. If -mcpu=future is used, GCC will use power10
> tuning. If you explicitly use -mtune=future, you will get a warning that
> -mtune=future is not supported, and default tuning will be set for power10.
>
> The patches have been tested on both little and big endian systems. Can I check
> it into the master branch?
>
> 2024-01-05 Michael Meissner <meissner@linux.ibm.com>
>
> gcc/
>
> * config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): Define
> __ARCH_PWR_FUTURE__ if -mcpu=future.
> * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS): New macro.
> (POWERPC_MASKS): Add -mcpu=future support.
> * config/rs6000/rs6000-opts.h (enum processor_type): Add
> PROCESSOR_FUTURE.
> * config/rs6000/rs6000-tables.opt: Regenerate.
> * config/rs6000/rs6000.cc (rs600_cpu_index_lookup): New helper
> function.
> (rs6000_option_override_internal): Make -mcpu=future set
> -mtune=power10. If the user explicitly uses -mtune=future, give a
> warning and reset the tuning to power10.
> (rs6000_option_override_internal): Use power10 costs for future
> machine.
> (rs6000_machine_from_flags): Add support for -mcpu=future.
> (rs6000_opt_masks): Likewise.
> * config/rs6000/rs6000.h (ASM_CPU_SUPPORT): Likewise.
> * config/rs6000/rs6000.md (cpu attribute): Likewise.
> * config/rs6000/rs6000.opt (-mfuture): New undocumented debug switch.
> * doc/invoke.texi (IBM RS/6000 and PowerPC Options): Document -mcpu=future.
> ---
> gcc/config/rs6000/rs6000-c.cc | 2 +
> gcc/config/rs6000/rs6000-cpus.def | 6 +++
> gcc/config/rs6000/rs6000-opts.h | 4 +-
> gcc/config/rs6000/rs6000-tables.opt | 3 ++
> gcc/config/rs6000/rs6000.cc | 58 ++++++++++++++++++++++++-----
> gcc/config/rs6000/rs6000.h | 1 +
> gcc/config/rs6000/rs6000.md | 2 +-
> gcc/config/rs6000/rs6000.opt | 4 ++
> gcc/doc/invoke.texi | 2 +-
> 9 files changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index ce0b14a8d37..f2fb5bef678 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ 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 ((flags & OPTION_MASK_SOFT_FLOAT) != 0)
> rs6000_define_or_undefine_macro (define_p, "_SOFT_FLOAT");
> if ((flags & OPTION_MASK_RECIP_PRECISION) != 0)
> diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
> index d28cc87eb2a..8754635f3d9 100644
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -88,6 +88,10 @@
> | OPTION_MASK_POWER10 \
> | OTHER_POWER10_MASKS)
>
> +/* Flags for a potential future processor that may or may not be delivered. */
> +#define ISA_FUTURE_MASKS (ISA_3_1_MASKS_SERVER \
> + | OPTION_MASK_FUTURE)
> +
Nit: Named as "ISA_FUTURE_MASKS_SERVER" seems more accurate as it's constituted
with ISA_3_1_MASKS_**SERVER** ...
> /* Flags that need to be turned off if -mno-power9-vector. */
> #define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW \
> | OPTION_MASK_P9_MINMAX)
> @@ -135,6 +139,7 @@
> | OPTION_MASK_LOAD_VECTOR_PAIR \
> | OPTION_MASK_POWER10 \
> | OPTION_MASK_P10_FUSION \
> + | OPTION_MASK_FUTURE \
> | OPTION_MASK_HTM \
> | OPTION_MASK_ISEL \
> | OPTION_MASK_MFCRF \
> @@ -267,3 +272,4 @@ RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, OPTION_MASK_PPC_GFXOPT
> RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64
> | ISA_2_7_MASKS_SERVER | OPTION_MASK_HTM)
> RS6000_CPU ("rs64", PROCESSOR_RS64A, OPTION_MASK_PPC_GFXOPT | MASK_POWERPC64)
> +RS6000_CPU ("future", PROCESSOR_FUTURE, MASK_POWERPC64 | ISA_FUTURE_MASKS)
..., then this need to be updated accordingly.
> diff --git a/gcc/config/rs6000/rs6000-opts.h b/gcc/config/rs6000/rs6000-opts.h
> index 33fd0efc936..25890ae3034 100644
> --- a/gcc/config/rs6000/rs6000-opts.h
> +++ b/gcc/config/rs6000/rs6000-opts.h
> @@ -67,7 +67,9 @@ enum processor_type
> PROCESSOR_MPCCORE,
> PROCESSOR_CELL,
> PROCESSOR_PPCA2,
> - PROCESSOR_TITAN
> + PROCESSOR_TITAN,
> +
Nit: unintentional empty line?
> + PROCESSOR_FUTURE
> };
>
>
> diff --git a/gcc/config/rs6000/rs6000-tables.opt b/gcc/config/rs6000/rs6000-tables.opt
> index 65f46709716..97fa98a2e65 100644
> --- a/gcc/config/rs6000/rs6000-tables.opt
> +++ b/gcc/config/rs6000/rs6000-tables.opt
> @@ -197,3 +197,6 @@ Enum(rs6000_cpu_opt_value) String(powerpc64le) Value(55)
> EnumValue
> Enum(rs6000_cpu_opt_value) String(rs64) Value(56)
>
> +EnumValue
> +Enum(rs6000_cpu_opt_value) String(future) Value(57)
> +
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 5a7e00b03d1..bc509399cf6 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -1809,6 +1809,18 @@ rs6000_cpu_name_lookup (const char *name)
> return -1;
> }
>
> +/* Look up the index for a specific processor. */
> +
> +static int
> +rs600_cpu_index_lookup (enum processor_type processor)
s/rs600_cpu_index_lookup/rs6000_cpu_index_lookup/
> +{
> + for (size_t i = 0; i < ARRAY_SIZE (processor_target_table); i++)
> + if (processor_target_table[i].processor == processor)
> + return i;
> +
> + return -1;
> +}
Nit: Since this is given with a valid enum processor_type, I think it should
never return -1? If so, may be more clear with gcc_unreachable () or adjust
with initial -1, break when hits and assert it's not -1.
> +
>
> /* Return number of consecutive hard regs needed starting at reg REGNO
> to hold something of mode MODE.
> @@ -3756,23 +3768,45 @@ rs6000_option_override_internal (bool global_init_p)
> rs6000_isa_flags &= ~OPTION_MASK_POWERPC64;
> #endif
>
> + /* At the moment, we don't have explict -mtune=future support. If the user
Nit: s/explict/explicit/
> + explicitly tried to use -mtune=future, give a warning. If not, use the
Nit: s/tried/tries/?
> + power10 tuning until future tuning is added. */
> if (rs6000_tune_index >= 0)
> - tune_index = rs6000_tune_index;
> + {
> + enum processor_type cur_proc
> + = processor_target_table[rs6000_tune_index].processor;
> +
> + if (cur_proc == PROCESSOR_FUTURE)
> + {
> + static bool issued_future_tune_warning = false;
> + if (!issued_future_tune_warning)
> + {
> + issued_future_tune_warning = true;
This seems to ensure we only warn this once, but I noticed that in rs6000/
only some OPT_Wpsabi related warnings adopt this way, I wonder if we don't
restrict it like this, for a tiny simple case, how many times it would warn?
> + warning (0, "%qs is not currently supported", "-mtune=future");
> + }
> +> + rs6000_tune_index = rs600_cpu_index_lookup (PROCESSOR_POWER10);
> + }
> + tune_index = rs6000_tune_index;
> + }
> else if (cpu_index >= 0)
> - rs6000_tune_index = tune_index = cpu_index;
> + {
> + enum processor_type cur_cpu
> + = processor_target_table[cpu_index].processor;
> +
> + rs6000_tune_index = tune_index
> + = (cur_cpu == PROCESSOR_FUTURE
> + ? rs600_cpu_index_lookup (PROCESSOR_POWER10)
s/rs600_cpu_index_lookup/rs6000_cpu_index_lookup/
> + : cpu_index);
> + }
> 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;
> - }
> + tune_index = rs600_cpu_index_lookup (tune_proc == PROCESSOR_FUTURE
> + ? PROCESSOR_POWER10
> + : tune_proc);
This part looks useless, as tune_proc is impossible to be PROCESSOR_FUTURE.
> }
Maybe re-structure the above into:
bool explicit_tune = false;
if (rs6000_tune_index >= 0)
{
tune_index = rs6000_tune_index;
explicit_tune = true;
}
else if (cpu_index >= 0)
// as before
rs6000_tune_index = tune_index = cpu_index;
else
{
//as before
...
}
// Check tune_index here instead.
if (processor_target_table[tune_index].processor == PROCESSOR_FUTURE)
{
tune_index = rs6000_cpu_index_lookup (PROCESSOR_POWER10);
if (explicit_tune)
warn ...
}
// as before
rs6000_tune = processor_target_table[tune_index].processor;
>
> if (cpu_index >= 0)
> @@ -4785,6 +4819,7 @@ rs6000_option_override_internal (bool global_init_p)
> break;
>
> case PROCESSOR_POWER10:
> + case PROCESSOR_FUTURE:
> rs6000_cost = &power10_cost;
> break;
>
> @@ -5944,6 +5979,8 @@ rs6000_machine_from_flags (void)
> /* Disable the flags that should never influence the .machine selection. */
> flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ISEL);
>
> + if ((flags & (ISA_FUTURE_MASKS & ~ISA_3_1_MASKS_SERVER)) != 0)
> + return "future";
> if ((flags & (ISA_3_1_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0)
> return "power10";
> if ((flags & (ISA_3_0_MASKS_SERVER & ~ISA_2_7_MASKS_SERVER)) != 0)
> @@ -24500,6 +24537,7 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] =
> { "float128-hardware", OPTION_MASK_FLOAT128_HW, false, true },
> { "fprnd", OPTION_MASK_FPRND, false, true },
> { "power10", OPTION_MASK_POWER10, false, true },
> + { "future", OPTION_MASK_FUTURE, false, true },
> { "hard-dfp", OPTION_MASK_DFP, false, true },
> { "htm", OPTION_MASK_HTM, false, true },
> { "isel", OPTION_MASK_ISEL, false, true },
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 2291fe8d3a3..43209f9a6e7 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -163,6 +163,7 @@
> mcpu=e5500: -me5500; \
> mcpu=e6500: -me6500; \
> mcpu=titan: -mtitan; \
> + mcpu=future: -mfuture; \
> !mcpu*: %{mpower9-vector: -mpower9; \
> mpower8-vector|mcrypto|mdirect-move|mhtm: -mpower8; \
> mvsx: -mpower7; \
I think we should also update asm_names in driver-rs6000.cc.
The others look good to me, thanks!
BR,
Kewen
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 969d34b69e6..a125fd8fc99 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -351,7 +351,7 @@ (define_attr "cpu"
> ppc403,ppc405,ppc440,ppc476,
> ppc8540,ppc8548,ppce300c2,ppce300c3,ppce500mc,ppce500mc64,ppce5500,ppce6500,
> power4,power5,power6,power7,power8,power9,power10,
> - rs64a,mpccore,cell,ppca2,titan"
> + rs64a,mpccore,cell,ppca2,titan,future"
> (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
>
> ;; The ISA we implement.
> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
> index 60b923f5e4b..775ba830eac 100644
> --- a/gcc/config/rs6000/rs6000.opt
> +++ b/gcc/config/rs6000/rs6000.opt
> @@ -628,6 +628,10 @@ mieee128-constant
> Target Var(TARGET_IEEE128_CONSTANT) Init(1) Save
> Generate (do not generate) code that uses the LXVKQ instruction.
>
> +mfuture
> +Target Undocumented Mask(FUTURE) Var(rs6000_isa_flags)
> +Generate (do not generate) future instructions.
> +
> ; Documented parameters
>
> -param=rs6000-vect-unroll-limit=
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index d71583853f0..0e817ee923a 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -30423,7 +30423,7 @@ Supported values for @var{cpu_type} are @samp{401}, @samp{403},
> @samp{titan}, @samp{power3}, @samp{power4}, @samp{power5}, @samp{power5+},
> @samp{power6}, @samp{power6x}, @samp{power7}, @samp{power8},
> @samp{power9}, @samp{power10}, @samp{powerpc}, @samp{powerpc64},
> -@samp{powerpc64le}, @samp{rs64}, and @samp{native}.
> +@samp{powerpc64le}, @samp{rs64}, @samp{future}, and @samp{native}.
>
> @option{-mcpu=powerpc}, @option{-mcpu=powerpc64}, and
> @option{-mcpu=powerpc64le} specify pure 32-bit PowerPC (either
On Tue, Jan 23, 2024 at 04:44:32PM +0800, Kewen.Lin wrote:
> > --- a/gcc/config/rs6000/rs6000-cpus.def
> > +++ b/gcc/config/rs6000/rs6000-cpus.def
> > @@ -88,6 +88,10 @@
> > | OPTION_MASK_POWER10 \
> > | OTHER_POWER10_MASKS)
> >
> > +/* Flags for a potential future processor that may or may not be delivered. */
> > +#define ISA_FUTURE_MASKS (ISA_3_1_MASKS_SERVER \
> > + | OPTION_MASK_FUTURE)
> > +
>
> Nit: Named as "ISA_FUTURE_MASKS_SERVER" seems more accurate as it's constituted
> with ISA_3_1_MASKS_**SERVER** ...
Well the _SERVER stuff was due to the power7 days when we still had to support
the E500 in the main rs6000 tree. But I will change it to be more consistant
in the future patches.
> ..., then this need to be updated accordingly.
>
> > diff --git a/gcc/config/rs6000/rs6000-opts.h b/gcc/config/rs6000/rs6000-opts.h
> > index 33fd0efc936..25890ae3034 100644
> > --- a/gcc/config/rs6000/rs6000-opts.h
> > +++ b/gcc/config/rs6000/rs6000-opts.h
> > @@ -67,7 +67,9 @@ enum processor_type
> > PROCESSOR_MPCCORE,
> > PROCESSOR_CELL,
> > PROCESSOR_PPCA2,
> > - PROCESSOR_TITAN
> > + PROCESSOR_TITAN,
> > +
>
> Nit: unintentional empty line?
>
> > + PROCESSOR_FUTURE
> > };
It was more as a separation. The MPCCORE, CELL, PPCA2, and TITAN are rather
old processors. I don't recall why we kept them after the POWER<x>.
Logically we should re-order the list and move MPCCORE, etc. earlier, but I
will delete the blank line in future patches.
> > +static int
> > +rs600_cpu_index_lookup (enum processor_type processor)
>
> s/rs600_cpu_index_lookup/rs6000_cpu_index_lookup/
I'm going to redo it, and eliminate rs600_cpu_index_lookup. Thanks for
catching the spelling of rs600 instead of rs6000.
> > +{
> > + for (size_t i = 0; i < ARRAY_SIZE (processor_target_table); i++)
> > + if (processor_target_table[i].processor == processor)
> > + return i;
> > +
> > + return -1;
> > +}
>
> Nit: Since this is given with a valid enum processor_type, I think it should
> never return -1? If so, may be more clear with gcc_unreachable () or adjust
> with initial -1, break when hits and assert it's not -1.
As I said, in looking at it, I think I will rewrite the code that uses it to
call rs6000_cpu_name_lookup instead.
> > +
> >
> > /* Return number of consecutive hard regs needed starting at reg REGNO
> > to hold something of mode MODE.
> > @@ -3756,23 +3768,45 @@ rs6000_option_override_internal (bool global_init_p)
> > rs6000_isa_flags &= ~OPTION_MASK_POWERPC64;
> > #endif
> >
> > + /* At the moment, we don't have explict -mtune=future support. If the user
>
> Nit: s/explict/explicit/
Thanks.
>
> > + explicitly tried to use -mtune=future, give a warning. If not, use the
>
> Nit: s/tried/tries/?
Thanks. I will reword the comment.
> > + power10 tuning until future tuning is added. */
> > if (rs6000_tune_index >= 0)
> > - tune_index = rs6000_tune_index;
> > + {
> > + enum processor_type cur_proc
> > + = processor_target_table[rs6000_tune_index].processor;
> > +
> > + if (cur_proc == PROCESSOR_FUTURE)
> > + {
> > + static bool issued_future_tune_warning = false;
> > + if (!issued_future_tune_warning)
> > + {
> > + issued_future_tune_warning = true;
>
> This seems to ensure we only warn this once, but I noticed that in rs6000/
> only some OPT_Wpsabi related warnings adopt this way, I wonder if we don't
> restrict it like this, for a tiny simple case, how many times it would warn?
In a simple case, you would only get the warning once. But if you use
__attribute__((__target__(...))) or #pragma target ... you might see it more
than once.
> > + warning (0, "%qs is not currently supported", "-mtune=future");
> > + }
> > +> + rs6000_tune_index = rs600_cpu_index_lookup (PROCESSOR_POWER10);
> > + }
> > + tune_index = rs6000_tune_index;
> > + }
> > else if (cpu_index >= 0)
> > - rs6000_tune_index = tune_index = cpu_index;
> > + {
> > + enum processor_type cur_cpu
> > + = processor_target_table[cpu_index].processor;
> > +
> > + rs6000_tune_index = tune_index
> > + = (cur_cpu == PROCESSOR_FUTURE
> > + ? rs600_cpu_index_lookup (PROCESSOR_POWER10)
>
> s/rs600_cpu_index_lookup/rs6000_cpu_index_lookup/
See above.
> > + : cpu_index);
> > + }
> > 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;
> > - }
> > + tune_index = rs600_cpu_index_lookup (tune_proc == PROCESSOR_FUTURE
> > + ? PROCESSOR_POWER10
> > + : tune_proc);
>
> This part looks useless, as tune_proc is impossible to be PROCESSOR_FUTURE.
Well in theory, you could configure the compiler with --with-cpu=future or
--with-tune=future.
> > }
>
> Maybe re-structure the above into:
>
> bool explicit_tune = false;
> if (rs6000_tune_index >= 0)
> {
> tune_index = rs6000_tune_index;
> explicit_tune = true;
> }
> else if (cpu_index >= 0)
> // as before
> rs6000_tune_index = tune_index = cpu_index;
> else
> {
> //as before
> ...
> }
>
> // Check tune_index here instead.
>
> if (processor_target_table[tune_index].processor == PROCESSOR_FUTURE)
> {
> tune_index = rs6000_cpu_index_lookup (PROCESSOR_POWER10);
> if (explicit_tune)
> warn ...
> }
>
> // as before
> rs6000_tune = processor_target_table[tune_index].processor;
>
> >
> > if (cpu_index >= 0)
> > @@ -4785,6 +4819,7 @@ rs6000_option_override_internal (bool global_init_p)
> > break;
> >
> > case PROCESSOR_POWER10:
> > + case PROCESSOR_FUTURE:
> > rs6000_cost = &power10_cost;
> > break;
> >
> > @@ -5944,6 +5979,8 @@ rs6000_machine_from_flags (void)
> > /* Disable the flags that should never influence the .machine selection. */
> > flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ISEL);
> >
> > + if ((flags & (ISA_FUTURE_MASKS & ~ISA_3_1_MASKS_SERVER)) != 0)
> > + return "future";
> > if ((flags & (ISA_3_1_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0)
> > return "power10";
> > if ((flags & (ISA_3_0_MASKS_SERVER & ~ISA_2_7_MASKS_SERVER)) != 0)
> > @@ -24500,6 +24537,7 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] =
> > { "float128-hardware", OPTION_MASK_FLOAT128_HW, false, true },
> > { "fprnd", OPTION_MASK_FPRND, false, true },
> > { "power10", OPTION_MASK_POWER10, false, true },
> > + { "future", OPTION_MASK_FUTURE, false, true },
> > { "hard-dfp", OPTION_MASK_DFP, false, true },
> > { "htm", OPTION_MASK_HTM, false, true },
> > { "isel", OPTION_MASK_ISEL, false, true },
> > diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> > index 2291fe8d3a3..43209f9a6e7 100644
> > --- a/gcc/config/rs6000/rs6000.h
> > +++ b/gcc/config/rs6000/rs6000.h
> > @@ -163,6 +163,7 @@
> > mcpu=e5500: -me5500; \
> > mcpu=e6500: -me6500; \
> > mcpu=titan: -mtitan; \
> > + mcpu=future: -mfuture; \
> > !mcpu*: %{mpower9-vector: -mpower9; \
> > mpower8-vector|mcrypto|mdirect-move|mhtm: -mpower8; \
> > mvsx: -mpower7; \
>
> I think we should also update asm_names in driver-rs6000.cc.
Ok. Though the driver-rs6000.cc stuff won't kick in until we have a real
system that matches "future".
on 2024/2/6 14:01, Michael Meissner wrote:
> On Tue, Jan 23, 2024 at 04:44:32PM +0800, Kewen.Lin wrote:
...
>>> diff --git a/gcc/config/rs6000/rs6000-opts.h b/gcc/config/rs6000/rs6000-opts.h
>>> index 33fd0efc936..25890ae3034 100644
>>> --- a/gcc/config/rs6000/rs6000-opts.h
>>> +++ b/gcc/config/rs6000/rs6000-opts.h
>>> @@ -67,7 +67,9 @@ enum processor_type
>>> PROCESSOR_MPCCORE,
>>> PROCESSOR_CELL,
>>> PROCESSOR_PPCA2,
>>> - PROCESSOR_TITAN
>>> + PROCESSOR_TITAN,
>>> +
>>
>> Nit: unintentional empty line?
>>
>>> + PROCESSOR_FUTURE
>>> };
>
> It was more as a separation. The MPCCORE, CELL, PPCA2, and TITAN are rather
> old processors. I don't recall why we kept them after the POWER<x>.
>
> Logically we should re-order the list and move MPCCORE, etc. earlier, but I
> will delete the blank line in future patches.
Thanks for clarifying, the re-order thing can be done in a separate patch and
in this context one comment line would be better than a blank line. :)
...
>>> + power10 tuning until future tuning is added. */
>>> if (rs6000_tune_index >= 0)
>>> - tune_index = rs6000_tune_index;
>>> + {
>>> + enum processor_type cur_proc
>>> + = processor_target_table[rs6000_tune_index].processor;
>>> +
>>> + if (cur_proc == PROCESSOR_FUTURE)
>>> + {
>>> + static bool issued_future_tune_warning = false;
>>> + if (!issued_future_tune_warning)
>>> + {
>>> + issued_future_tune_warning = true;
>>
>> This seems to ensure we only warn this once, but I noticed that in rs6000/
>> only some OPT_Wpsabi related warnings adopt this way, I wonder if we don't
>> restrict it like this, for a tiny simple case, how many times it would warn?
>
> In a simple case, you would only get the warning once. But if you use
> __attribute__((__target__(...))) or #pragma target ... you might see it more
> than once.
OK, considering we only get this warning once for a simple case, I'm inclined
not to keep a static variable for it, it's the same as what we do currently
for option conflict errors emission. But I'm fine for either.
>>> 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;
>>> - }
>>> + tune_index = rs600_cpu_index_lookup (tune_proc == PROCESSOR_FUTURE
>>> + ? PROCESSOR_POWER10
>>> + : tune_proc);
>>
>> This part looks useless, as tune_proc is impossible to be PROCESSOR_FUTURE.
>
> Well in theory, you could configure the compiler with --with-cpu=future or
> --with-tune=future.
Sorry for the possible confusion here, the "tune_proc" that I referred to is
the variable in the above else branch:
enum processor_type tune_proc = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
It's either PROCESSOR_DEFAULT64 or PROCESSOR_DEFAULT, so it doesn't have a
chance to be PROCESSOR_FUTURE, so the checking "tune_proc == PROCESSOR_FUTURE"
is useless.
That's why I suggested the below flow, it does a final check out of those checks,
it looks a bit more clear IMHO.
>
>>> }
>>
>> Maybe re-structure the above into:
>>
>> bool explicit_tune = false;
>> if (rs6000_tune_index >= 0)
>> {
>> tune_index = rs6000_tune_index;
>> explicit_tune = true;
>> }
>> else if (cpu_index >= 0)
>> // as before
>> rs6000_tune_index = tune_index = cpu_index;
>> else
>> {
>> //as before
>> ...
>> }
>>
>> // Check tune_index here instead.
>>
>> if (processor_target_table[tune_index].processor == PROCESSOR_FUTURE)
>> {
>> tune_index = rs6000_cpu_index_lookup (PROCESSOR_POWER10);
>> if (explicit_tune)
>> warn ...
>> }
>>
>> // as before
>> rs6000_tune = processor_target_table[tune_index].processor;
>>
>>>
BR,
Kewen
On Wed, Feb 07, 2024 at 05:21:10PM +0800, Kewen.Lin wrote:
> on 2024/2/6 14:01, Michael Meissner wrote:
> Sorry for the possible confusion here, the "tune_proc" that I referred to is
> the variable in the above else branch:
>
> enum processor_type tune_proc = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
>
> It's either PROCESSOR_DEFAULT64 or PROCESSOR_DEFAULT, so it doesn't have a
> chance to be PROCESSOR_FUTURE, so the checking "tune_proc == PROCESSOR_FUTURE"
> is useless.
PROCESSOR_DEFAULT can be PROCESSOR_FUTURE if somebody configures GCC with
--with-cpu=future. While in general it shouldn't occur, it is helpful to
consider all of the corner cases.
On Tue, Feb 06, 2024 at 01:01:52AM -0500, Michael Meissner wrote:
> > Nit: Named as "ISA_FUTURE_MASKS_SERVER" seems more accurate as it's constituted
> > with ISA_3_1_MASKS_**SERVER** ...
>
> Well the _SERVER stuff was due to the power7 days when we still had to support
> the E500 in the main rs6000 tree. But I will change it to be more consistant
> in the future patches.
"_SERVER" still is a good shortish name for the server systems ;-)
> > > @@ -67,7 +67,9 @@ enum processor_type
> > > PROCESSOR_MPCCORE,
> > > PROCESSOR_CELL,
> > > PROCESSOR_PPCA2,
> > > - PROCESSOR_TITAN
> > > + PROCESSOR_TITAN,
> > > +
> >
> > Nit: unintentional empty line?
> >
> > > + PROCESSOR_FUTURE
> > > };
>
> It was more as a separation. The MPCCORE, CELL, PPCA2, and TITAN are rather
> old processors. I don't recall why we kept them after the POWER<x>.
Please don't add random separations.
> Logically we should re-order the list and move MPCCORE, etc. earlier, but I
> will delete the blank line in future patches.
Don't randomly reorder, either.
_FUTURE should be added after POWER11.
> > I think we should also update asm_names in driver-rs6000.cc.
>
> Ok. Though the driver-rs6000.cc stuff won't kick in until we have a real
> system that matches "future".
Or when during development you have that faked. You did test it, right?
:-)
Segher
On Wed, Feb 07, 2024 at 05:21:10PM +0800, Kewen.Lin wrote:
> on 2024/2/6 14:01, Michael Meissner wrote:
> > It was more as a separation. The MPCCORE, CELL, PPCA2, and TITAN are rather
> > old processors.
I'll probably remove Titan soonish, btw. We have adjusted code around
it for what, fifteen years? But the hardware never materialized. There
are more silly things in our backend, but this one takes the prize.
> OK, considering we only get this warning once for a simple case, I'm inclined
> not to keep a static variable for it, it's the same as what we do currently
> for option conflict errors emission. But I'm fine for either.
Whatever is easiest.
Segher
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
Hi Mike,
Sorry for late reply (just back from vacation).
on 2024/2/8 03:58, Michael Meissner wrote:
> On Wed, Feb 07, 2024 at 05:21:10PM +0800, Kewen.Lin wrote:
>> on 2024/2/6 14:01, Michael Meissner wrote:
>> Sorry for the possible confusion here, the "tune_proc" that I referred to is
>> the variable in the above else branch:
>>
>> enum processor_type tune_proc = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
>>
>> It's either PROCESSOR_DEFAULT64 or PROCESSOR_DEFAULT, so it doesn't have a
>> chance to be PROCESSOR_FUTURE, so the checking "tune_proc == PROCESSOR_FUTURE"
>> is useless.
>
> PROCESSOR_DEFAULT can be PROCESSOR_FUTURE if somebody configures GCC with
> --with-cpu=future. While in general it shouldn't occur, it is helpful to
> consider all of the corner cases.
But it sounds not true, I think you meant TARGET_CPU_DEFAULT instead?
On one local ppc64le machine I tried to configure with --with-cpu=power10,
I got {,OPTION_}TARGET_CPU_DEFAULT "power10" but PROCESSOR_DEFAULT is still
PROCESSOR_POWER7 (PROCESSOR_DEFAULT64 is PROCESSOR_POWER8). I think these
PROCESSOR_DEFAULT{,64} are defined by various headers:
$ grep -r "define PROCESSOR_DEFAULT" gcc/config/rs6000/
gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER8
gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7400
gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4
gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7450
gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC603
gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT64 PROCESSOR_RS64A
gcc/config/rs6000/vxworks.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC604
, and they are unlikely to be updated later, no?
btw, the given --with-cpu=future will make cpu_index never negative so
...
else if (cpu_index >= 0)
rs6000_tune_index = tune_index = cpu_index;
else
...
so there is no chance to enter "else" arm, that is, that arm only takes
effect when no cpu/tune is given (neither -m{cpu,tune} nor --with-cpu=).
BR,
Kewen
On Tue, Feb 20, 2024 at 06:35:34PM +0800, Kewen.Lin wrote:
> Hi Mike,
>
> Sorry for late reply (just back from vacation).
>
> on 2024/2/8 03:58, Michael Meissner wrote:
> > On Wed, Feb 07, 2024 at 05:21:10PM +0800, Kewen.Lin wrote:
> >> on 2024/2/6 14:01, Michael Meissner wrote:
> >> Sorry for the possible confusion here, the "tune_proc" that I referred to is
> >> the variable in the above else branch:
> >>
> >> enum processor_type tune_proc = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
> >>
> >> It's either PROCESSOR_DEFAULT64 or PROCESSOR_DEFAULT, so it doesn't have a
> >> chance to be PROCESSOR_FUTURE, so the checking "tune_proc == PROCESSOR_FUTURE"
> >> is useless.
> >
> > PROCESSOR_DEFAULT can be PROCESSOR_FUTURE if somebody configures GCC with
> > --with-cpu=future. While in general it shouldn't occur, it is helpful to
> > consider all of the corner cases.
>
> But it sounds not true, I think you meant TARGET_CPU_DEFAULT instead?
>
> On one local ppc64le machine I tried to configure with --with-cpu=power10,
> I got {,OPTION_}TARGET_CPU_DEFAULT "power10" but PROCESSOR_DEFAULT is still
> PROCESSOR_POWER7 (PROCESSOR_DEFAULT64 is PROCESSOR_POWER8). I think these
> PROCESSOR_DEFAULT{,64} are defined by various headers:
Yes, I was mistaken. You are correct TARGET_CPU_DEFAULT is set. I will change
the comments.
> gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
> gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
> gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
> gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
> gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER8
> gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
> gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7400
> gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4
> gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7450
> gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
> gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
> gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
> gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC603
> gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT64 PROCESSOR_RS64A
> gcc/config/rs6000/vxworks.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC604
>
> , and they are unlikely to be updated later, no?
>
> btw, the given --with-cpu=future will make cpu_index never negative so
>
> ...
> else if (cpu_index >= 0)
> rs6000_tune_index = tune_index = cpu_index;
> else
> ...
>
> so there is no chance to enter "else" arm, that is, that arm only takes
> effect when no cpu/tune is given (neither -m{cpu,tune} nor --with-cpu=).
Note, this is existing code. I didn't modify it. If we want to change it, we
should do it as another patch.
On Tue, Feb 20, 2024 at 06:35:34PM +0800, Kewen.Lin wrote:
> on 2024/2/8 03:58, Michael Meissner wrote:
> $ grep -r "define PROCESSOR_DEFAULT" gcc/config/rs6000/
> gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
> gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
> gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
> gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
> gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER8
> gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
> gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7400
> gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4
> gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7450
> gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
> gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
> gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
> gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC603
> gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT64 PROCESSOR_RS64A
> gcc/config/rs6000/vxworks.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC604
>
> , and they are unlikely to be updated later, no?
In most cases did would be an ABI change. Almost never an acceptable
thing to do.
Segher
on 2024/2/21 15:19, Michael Meissner wrote:
> On Tue, Feb 20, 2024 at 06:35:34PM +0800, Kewen.Lin wrote:
>> Hi Mike,
>>
>> Sorry for late reply (just back from vacation).
>>
>> on 2024/2/8 03:58, Michael Meissner wrote:
>>> On Wed, Feb 07, 2024 at 05:21:10PM +0800, Kewen.Lin wrote:
>>>> on 2024/2/6 14:01, Michael Meissner wrote:
>>>> Sorry for the possible confusion here, the "tune_proc" that I referred to is
>>>> the variable in the above else branch:
>>>>
>>>> enum processor_type tune_proc = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
>>>>
>>>> It's either PROCESSOR_DEFAULT64 or PROCESSOR_DEFAULT, so it doesn't have a
>>>> chance to be PROCESSOR_FUTURE, so the checking "tune_proc == PROCESSOR_FUTURE"
>>>> is useless.
>>>
>>> PROCESSOR_DEFAULT can be PROCESSOR_FUTURE if somebody configures GCC with
>>> --with-cpu=future. While in general it shouldn't occur, it is helpful to
>>> consider all of the corner cases.
>>
>> But it sounds not true, I think you meant TARGET_CPU_DEFAULT instead?
>>
>> On one local ppc64le machine I tried to configure with --with-cpu=power10,
>> I got {,OPTION_}TARGET_CPU_DEFAULT "power10" but PROCESSOR_DEFAULT is still
>> PROCESSOR_POWER7 (PROCESSOR_DEFAULT64 is PROCESSOR_POWER8). I think these
>> PROCESSOR_DEFAULT{,64} are defined by various headers:
>
> Yes, I was mistaken. You are correct TARGET_CPU_DEFAULT is set. I will change
> the comments.
Thanks!
>
>> gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
>> gcc/config/rs6000/aix71.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
>> gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
>> gcc/config/rs6000/aix72.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER7
>> gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER8
>> gcc/config/rs6000/aix73.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
>> gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7400
>> gcc/config/rs6000/darwin.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER4
>> gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC7450
>> gcc/config/rs6000/freebsd64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
>> gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT PROCESSOR_POWER7
>> gcc/config/rs6000/linux64.h:#define PROCESSOR_DEFAULT64 PROCESSOR_POWER8
>> gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC603
>> gcc/config/rs6000/rs6000.h:#define PROCESSOR_DEFAULT64 PROCESSOR_RS64A
>> gcc/config/rs6000/vxworks.h:#define PROCESSOR_DEFAULT PROCESSOR_PPC604
>>
>> , and they are unlikely to be updated later, no?
>>
>> btw, the given --with-cpu=future will make cpu_index never negative so
>>
>> ...
>> else if (cpu_index >= 0)
>> rs6000_tune_index = tune_index = cpu_index;
>> else
>> ...
>>
>> so there is no chance to enter "else" arm, that is, that arm only takes
>> effect when no cpu/tune is given (neither -m{cpu,tune} nor --with-cpu=).
>
> Note, this is existing code. I didn't modify it. If we want to change it, we
> should do it as another patch.
Yes, I agree. Just to clarify, I didn't suggest changing it but instead
suggested almost keeping them, since we don't need any changes in "else"
arm, so instead of updating in arms "if" and "else if" for "future cpu type",
it seems a bit more clear to just check it after this, ie.:
----
bool explicit_tune = false;
if (rs6000_tune_index >= 0)
{
tune_index = rs6000_tune_index;
explicit_tune = true;
}
else if (cpu_index >= 0)
// as before
rs6000_tune_index = tune_index = cpu_index;
else
{
//as before
...
}
// Check tune_index here instead.
if (processor_target_table[tune_index].processor == PROCESSOR_FUTURE)
{
tune_index = rs6000_cpu_index_lookup (PROCESSOR_POWER10);
if (explicit_tune)
warn ...
}
// as before
rs6000_tune = processor_target_table[tune_index].processor;
----
, copied from previous comment: https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643681.html
BR,
Kewen
@@ -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 ((flags & OPTION_MASK_SOFT_FLOAT) != 0)
rs6000_define_or_undefine_macro (define_p, "_SOFT_FLOAT");
if ((flags & OPTION_MASK_RECIP_PRECISION) != 0)
@@ -88,6 +88,10 @@
| OPTION_MASK_POWER10 \
| OTHER_POWER10_MASKS)
+/* Flags for a potential future processor that may or may not be delivered. */
+#define ISA_FUTURE_MASKS (ISA_3_1_MASKS_SERVER \
+ | OPTION_MASK_FUTURE)
+
/* Flags that need to be turned off if -mno-power9-vector. */
#define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW \
| OPTION_MASK_P9_MINMAX)
@@ -135,6 +139,7 @@
| OPTION_MASK_LOAD_VECTOR_PAIR \
| OPTION_MASK_POWER10 \
| OPTION_MASK_P10_FUSION \
+ | OPTION_MASK_FUTURE \
| OPTION_MASK_HTM \
| OPTION_MASK_ISEL \
| OPTION_MASK_MFCRF \
@@ -267,3 +272,4 @@ RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, OPTION_MASK_PPC_GFXOPT
RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64
| ISA_2_7_MASKS_SERVER | OPTION_MASK_HTM)
RS6000_CPU ("rs64", PROCESSOR_RS64A, OPTION_MASK_PPC_GFXOPT | MASK_POWERPC64)
+RS6000_CPU ("future", PROCESSOR_FUTURE, MASK_POWERPC64 | ISA_FUTURE_MASKS)
@@ -67,7 +67,9 @@ enum processor_type
PROCESSOR_MPCCORE,
PROCESSOR_CELL,
PROCESSOR_PPCA2,
- PROCESSOR_TITAN
+ PROCESSOR_TITAN,
+
+ PROCESSOR_FUTURE
};
@@ -197,3 +197,6 @@ Enum(rs6000_cpu_opt_value) String(powerpc64le) Value(55)
EnumValue
Enum(rs6000_cpu_opt_value) String(rs64) Value(56)
+EnumValue
+Enum(rs6000_cpu_opt_value) String(future) Value(57)
+
@@ -1809,6 +1809,18 @@ rs6000_cpu_name_lookup (const char *name)
return -1;
}
+/* Look up the index for a specific processor. */
+
+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;
+}
+
/* Return number of consecutive hard regs needed starting at reg REGNO
to hold something of mode MODE.
@@ -3756,23 +3768,45 @@ rs6000_option_override_internal (bool global_init_p)
rs6000_isa_flags &= ~OPTION_MASK_POWERPC64;
#endif
+ /* At the moment, we don't have explict -mtune=future support. If the user
+ explicitly tried to use -mtune=future, give a warning. If not, use the
+ power10 tuning until future tuning is added. */
if (rs6000_tune_index >= 0)
- tune_index = rs6000_tune_index;
+ {
+ enum processor_type cur_proc
+ = processor_target_table[rs6000_tune_index].processor;
+
+ if (cur_proc == PROCESSOR_FUTURE)
+ {
+ static bool issued_future_tune_warning = false;
+ if (!issued_future_tune_warning)
+ {
+ issued_future_tune_warning = true;
+ warning (0, "%qs is not currently supported", "-mtune=future");
+ }
+
+ rs6000_tune_index = rs600_cpu_index_lookup (PROCESSOR_POWER10);
+ }
+ tune_index = rs6000_tune_index;
+ }
else if (cpu_index >= 0)
- rs6000_tune_index = tune_index = cpu_index;
+ {
+ enum processor_type cur_cpu
+ = processor_target_table[cpu_index].processor;
+
+ rs6000_tune_index = tune_index
+ = (cur_cpu == PROCESSOR_FUTURE
+ ? rs600_cpu_index_lookup (PROCESSOR_POWER10)
+ : cpu_index);
+ }
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;
- }
+ tune_index = rs600_cpu_index_lookup (tune_proc == PROCESSOR_FUTURE
+ ? PROCESSOR_POWER10
+ : tune_proc);
}
if (cpu_index >= 0)
@@ -4785,6 +4819,7 @@ rs6000_option_override_internal (bool global_init_p)
break;
case PROCESSOR_POWER10:
+ case PROCESSOR_FUTURE:
rs6000_cost = &power10_cost;
break;
@@ -5944,6 +5979,8 @@ rs6000_machine_from_flags (void)
/* Disable the flags that should never influence the .machine selection. */
flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ISEL);
+ if ((flags & (ISA_FUTURE_MASKS & ~ISA_3_1_MASKS_SERVER)) != 0)
+ return "future";
if ((flags & (ISA_3_1_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0)
return "power10";
if ((flags & (ISA_3_0_MASKS_SERVER & ~ISA_2_7_MASKS_SERVER)) != 0)
@@ -24500,6 +24537,7 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] =
{ "float128-hardware", OPTION_MASK_FLOAT128_HW, false, true },
{ "fprnd", OPTION_MASK_FPRND, false, true },
{ "power10", OPTION_MASK_POWER10, false, true },
+ { "future", OPTION_MASK_FUTURE, false, true },
{ "hard-dfp", OPTION_MASK_DFP, false, true },
{ "htm", OPTION_MASK_HTM, false, true },
{ "isel", OPTION_MASK_ISEL, false, true },
@@ -163,6 +163,7 @@
mcpu=e5500: -me5500; \
mcpu=e6500: -me6500; \
mcpu=titan: -mtitan; \
+ mcpu=future: -mfuture; \
!mcpu*: %{mpower9-vector: -mpower9; \
mpower8-vector|mcrypto|mdirect-move|mhtm: -mpower8; \
mvsx: -mpower7; \
@@ -351,7 +351,7 @@ (define_attr "cpu"
ppc403,ppc405,ppc440,ppc476,
ppc8540,ppc8548,ppce300c2,ppce300c3,ppce500mc,ppce500mc64,ppce5500,ppce6500,
power4,power5,power6,power7,power8,power9,power10,
- rs64a,mpccore,cell,ppca2,titan"
+ rs64a,mpccore,cell,ppca2,titan,future"
(const (symbol_ref "(enum attr_cpu) rs6000_tune")))
;; The ISA we implement.
@@ -628,6 +628,10 @@ mieee128-constant
Target Var(TARGET_IEEE128_CONSTANT) Init(1) Save
Generate (do not generate) code that uses the LXVKQ instruction.
+mfuture
+Target Undocumented Mask(FUTURE) Var(rs6000_isa_flags)
+Generate (do not generate) future instructions.
+
; Documented parameters
-param=rs6000-vect-unroll-limit=
@@ -30423,7 +30423,7 @@ Supported values for @var{cpu_type} are @samp{401}, @samp{403},
@samp{titan}, @samp{power3}, @samp{power4}, @samp{power5}, @samp{power5+},
@samp{power6}, @samp{power6x}, @samp{power7}, @samp{power8},
@samp{power9}, @samp{power10}, @samp{powerpc}, @samp{powerpc64},
-@samp{powerpc64le}, @samp{rs64}, and @samp{native}.
+@samp{powerpc64le}, @samp{rs64}, @samp{future}, and @samp{native}.
@option{-mcpu=powerpc}, @option{-mcpu=powerpc64}, and
@option{-mcpu=powerpc64le} specify pure 32-bit PowerPC (either