Re: [PATCH-for-10.1 4/4] tcg: Convert TARGET_SUPPORTS_MTTCG to TCGCPUOps::mttcg_supported field

[Richard Henderson <richard.henderson@linaro.org>]

On 3/21/25 08:59, Philippe Mathieu-Daudé wrote:
> Instead of having a compile-time TARGET_SUPPORTS_MTTCG definition,
> have each target set the 'mttcg_supported' field in the TCGCPUOps
> structure.
> 
> Since so far we only emulate one target architecture at a time,
> tcg_init_machine() gets whether MTTCG is supported via the
> &first_cpu global.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   docs/devel/multi-thread-tcg.rst          | 2 +-
>   configs/targets/aarch64-softmmu.mak      | 1 -
>   configs/targets/alpha-softmmu.mak        | 1 -
>   configs/targets/arm-softmmu.mak          | 1 -
>   configs/targets/hppa-softmmu.mak         | 1 -
>   configs/targets/i386-softmmu.mak         | 1 -
>   configs/targets/loongarch64-softmmu.mak  | 1 -
>   configs/targets/microblaze-softmmu.mak   | 1 -
>   configs/targets/microblazeel-softmmu.mak | 1 -
>   configs/targets/mips-softmmu.mak         | 1 -
>   configs/targets/mipsel-softmmu.mak       | 1 -
>   configs/targets/or1k-softmmu.mak         | 1 -
>   configs/targets/ppc64-softmmu.mak        | 1 -
>   configs/targets/riscv32-softmmu.mak      | 1 -
>   configs/targets/riscv64-softmmu.mak      | 1 -
>   configs/targets/s390x-softmmu.mak        | 1 -
>   configs/targets/sparc-softmmu.mak        | 1 -
>   configs/targets/sparc64-softmmu.mak      | 1 -
>   configs/targets/x86_64-softmmu.mak       | 1 -
>   configs/targets/xtensa-softmmu.mak       | 1 -
>   configs/targets/xtensaeb-softmmu.mak     | 1 -
>   include/accel/tcg/cpu-ops.h              | 8 ++++++++
>   include/exec/poison.h                    | 1 -
>   accel/tcg/tcg-all.c                      | 7 ++-----
>   target/alpha/cpu.c                       | 1 +
>   target/arm/cpu.c                         | 1 +
>   target/arm/tcg/cpu-v7m.c                 | 1 +
>   target/avr/cpu.c                         | 1 +
>   target/hexagon/cpu.c                     | 1 +
>   target/hppa/cpu.c                        | 1 +
>   target/i386/tcg/tcg-cpu.c                | 1 +
>   target/loongarch/cpu.c                   | 1 +
>   target/m68k/cpu.c                        | 1 +
>   target/microblaze/cpu.c                  | 1 +
>   target/mips/cpu.c                        | 1 +
>   target/openrisc/cpu.c                    | 1 +
>   target/ppc/cpu_init.c                    | 1 +
>   target/riscv/tcg/tcg-cpu.c               | 1 +
>   target/rx/cpu.c                          | 1 +
>   target/s390x/cpu.c                       | 1 +
>   target/sh4/cpu.c                         | 1 +
>   target/sparc/cpu.c                       | 1 +
>   target/tricore/cpu.c                     | 1 +
>   target/xtensa/cpu.c                      | 1 +
>   44 files changed, 31 insertions(+), 27 deletions(-)

As far as this goes,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

> @@ -91,11 +92,7 @@ static int tcg_init_machine(MachineState *ms)
>   #else
>       unsigned max_cpus = ms->smp.max_cpus;
>   #endif
> -#ifdef TARGET_SUPPORTS_MTTCG
> -    bool mttcg_supported = true;
> -#else
> -    bool mttcg_supported = false;
> -#endif
> +    bool mttcg_supported = first_cpu->cc->tcg_ops->mttcg_supported; /* FIXME avoid first_cpu */

Intersection of CPU_FOREACH?

> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 24e52e28f44..9bc921c1cb2 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -216,6 +216,7 @@ static const TCGCPUOps avr_tcg_ops = {
>       .cpu_exec_halt = avr_cpu_has_work,
>       .tlb_fill = avr_cpu_tlb_fill,
>       .do_interrupt = avr_cpu_do_interrupt,
> +    .mttcg_supported = false,
>       .guest_default_memory_order = 0,
>   };

This may well be the only true not supported.
I'm not sure what needs to happen to make that true.

> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 34734b0edb0..7bcf297998f 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -324,6 +324,7 @@ static const TCGCPUOps hexagon_tcg_ops = {
>       .translate_code = hexagon_translate_code,
>       .synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
>       .restore_state_to_opc = hexagon_restore_state_to_opc,
> +    .mttcg_supported = false,
>       /* MTTCG not yet supported: require strict ordering */
>       .guest_default_memory_order = TCG_MO_ALL,

We now have strict ordering, right there with TCG_MO_ALL.
Likewise for m68k, sh4, tricore.
I think we simply forgot to update.

> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 5ddc9bbb829..eee604e9254 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -554,6 +554,7 @@ static const TCGCPUOps mips_tcg_ops = {
>       .synchronize_from_tb = mips_cpu_synchronize_from_tb,
>       .restore_state_to_opc = mips_restore_state_to_opc,
>   
> +    .mttcg_supported = TARGET_LONG_BITS == 32,
>       .guest_default_memory_order = 0,

What is missing from mip64?

> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 28fbbb8d3c1..ed79cc1a6b7 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7490,6 +7490,7 @@ static const TCGCPUOps ppc_tcg_ops = {
>     .translate_code = ppc_translate_code,
>     .restore_state_to_opc = ppc_restore_state_to_opc,
>   
> +  .mttcg_supported = TARGET_LONG_BITS == 64,
>     .guest_default_memory_order = 0,

Similarly.  I'd be surprised if ppc32 can't use mttcg, really.


r~