Re: [PATCH v2 03/50] tcg: Introduce tcg-global-mappings

On 7/29/26 20:09, Anton Johansson via qemu development wrote:
> +extern cpu_tcg_mapping tcg_global_mappings[];
> +extern size_t tcg_global_mapping_count;

No globals.  All such data structures would be local to the translators, calling

> +void init_cpu_tcg_mappings(cpu_tcg_mapping *mappings, size_t size);

as necessary.

> +void init_cpu_tcg_mappings(cpu_tcg_mapping *mappings, size_t size)
> +{
> +    uintptr_t tcg_addr;
> +    size_t cpu_offset;
> +    const char *name;
> +    cpu_tcg_mapping m;
> +
> +    /*
> +     * Paranoid assertion, this should always hold since
> +     * they're typedef'd to pointers. But you never know!
> +     */
> +    g_assert(sizeof(TCGv_i32) == sizeof(TCGv_i64));
> +
> +    /*
> +     * Loop over entries in tcg_global_mappings and
> +     * create the `mapped to` TCGv's.
> +     */
> +    for (int i = 0; i < size; ++i) {
> +        m = mappings[i];
> +
> +        for (int j = 0; j < m.number_of_elements; ++j) {
> +            /*
> +             * Here we are using the fact that
> +             * sizeof(TCGv_i32) == sizeof(TCGv_i64) == sizeof(TCGv)
> +             */
> +            assert(sizeof(TCGv_i32) == sizeof(TCGv_i64));

Two identical asserts in 10 lines.

Such things are better with a compile-time assert using QEMU_BUILD_BUG_ON, however...

> +            tcg_addr = (uintptr_t)m.tcg_var_base_address + j * sizeof(TCGv_i32);
> +            cpu_offset = m.cpu_var_base_offset + j * m.cpu_var_stride;
> +            name = m.cpu_var_names[j];
> +
> +            if (m.cpu_var_size < 8) {

I can understand wanting to use sizeof in the macros,
but we can do better than that with

   .type = _Generic(((struct_type *)0)->cpu_var,
                    uint64_t: TCG_TYPE_I64,
                    int64_t: TCG_TYPE_I64,
                    uint32_t: TCG_TYPE_I32,
                    int32_t: TCG_TYPE_I32),

which will catch mistakes earlier at compile-time.

> +                *(TCGv_i32 *)tcg_addr =
> +                    tcg_global_mem_new_i32(tcg_env, cpu_offset, name);
> +            } else {
> +                *(TCGv_i64 *)tcg_addr =
> +                    tcg_global_mem_new_i64(tcg_env, cpu_offset, name);
> +            }

At which point you can use tcg_global_mem_new_internal.


r~