Re: [PATCH] target-info: set target_arch statically

[Markus Armbruster <armbru@redhat.com>]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> target_arch() function will reparse target_name() every time if it was
> not set to a proper SYS_EMU_TARGET_* value (when using
> target-info-stub.c), which is not efficient.
>
> Since we want to preserve the constness of TargetInfo but C doesn't give
> us flexible compile time expressions, we simply set target_arch using a
> static constructor once instead.
>
> This was found when doing changes to virtio_access_is_big_endian()
> function, having an overhead of 50% after switching to runtime checks.
> With this, overhead left is around 3%, due to indirect function
> calls.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  target-info-stub.c | 11 ++++++++++-
>  target-info.c      |  9 +--------
>  2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/target-info-stub.c b/target-info-stub.c
> index 8392d81e8f8..ff86a02247a 100644
> --- a/target-info-stub.c
> +++ b/target-info-stub.c
> @@ -10,13 +10,14 @@
>  #include "qemu/target-info.h"
>  #include "qemu/target-info-impl.h"
>  #include "hw/core/boards.h"
> +#include "qapi/error.h"
>  #include "cpu.h"
>  
>  /* Validate correct placement of CPUArchState. */
>  QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
>  QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
>  
> -static const TargetInfo target_info_stub = {
> +static TargetInfo target_info_stub = {
>      .target_name = TARGET_NAME,
>      .target_arch = SYS_EMU_TARGET__MAX,
>      .long_bits = TARGET_LONG_BITS,
> @@ -29,3 +30,11 @@ const TargetInfo *target_info(void)
>  {
>      return &target_info_stub;
>  }
> +
> +__attribute__((constructor))
> +static void init_target_arch(void)
> +{
> +    target_info_stub.target_arch = qapi_enum_parse(&SysEmuTarget_lookup,
> +                                                   target_name(), -1,
> +                                                   &error_abort);
> +}

This is slightly unclean.  Constructors run before main().  If
qapi_enum_parse(..., &error_abort) fails here, error_handle() will
report the unexpected error with error_report() before main() calls
error_init().

See also

    From: Markus Armbruster <armbru@redhat.com>
    Subject: Re: [PATCH v3 02/20] monitor: initialize global data from a constructor 
    Date: Thu, 18 Sep 2025 08:30:23 +0200
    Message-ID: <87frck1dds.fsf@pond.sub.org>

Observation, not objection.

> diff --git a/target-info.c b/target-info.c
> index 24696ff4111..c3c0856d01a 100644
> --- a/target-info.c
> +++ b/target-info.c
> @@ -10,7 +10,6 @@
>  #include "qemu/target-info.h"
>  #include "qemu/target-info-qapi.h"
>  #include "qemu/target-info-impl.h"
> -#include "qapi/error.h"
>  
>  const char *target_name(void)
>  {
> @@ -24,13 +23,7 @@ unsigned target_long_bits(void)
>  
>  SysEmuTarget target_arch(void)
>  {
> -    SysEmuTarget arch = target_info()->target_arch;
> -
> -    if (arch == SYS_EMU_TARGET__MAX) {
> -        arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1,
> -                               &error_abort);
> -    }
> -    return arch;
> +    return target_info()->target_arch;
>  }
>  
>  const char *target_cpu_type(void)