Re: [Qemu-devel] [PATCH] cpu_defs: Simplify CPUTLB padding logic

[Paolo Bonzini]

On 05/07/2015 23:08, Peter Crosthwaite wrote:
> There was a complicated subtractive arithmetic for determining the
> padding on the CPUTLBEntry structure. Simplify this with a union.
> 
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>  include/exec/cpu-defs.h | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
> index 98b9cff..5093be2 100644
> --- a/include/exec/cpu-defs.h
> +++ b/include/exec/cpu-defs.h
> @@ -105,17 +105,18 @@ typedef struct CPUTLBEntry {
>         bit 3                      : indicates that the entry is invalid
>         bit 2..0                   : zero
>      */
> -    target_ulong addr_read;
> -    target_ulong addr_write;
> -    target_ulong addr_code;
> -    /* Addend to virtual address to get host address.  IO accesses
> -       use the corresponding iotlb value.  */
> -    uintptr_t addend;
> -    /* padding to get a power of two size */
> -    uint8_t dummy[(1 << CPU_TLB_ENTRY_BITS) -
> -                  (sizeof(target_ulong) * 3 +
> -                   ((-sizeof(target_ulong) * 3) & (sizeof(uintptr_t) - 1)) +
> -                   sizeof(uintptr_t))];
> +    union {

The struct CPUTLBEntry can be changed to union CPUTLBEntry directly,
with no need for the anonymous struct.

> +        struct {
> +            target_ulong addr_read;
> +            target_ulong addr_write;
> +            target_ulong addr_code;
> +            /* Addend to virtual address to get host address.  IO accesses
> +               use the corresponding iotlb value.  */
> +            uintptr_t addend;
> +        };

Which compiler version started implementing anonymous structs?

Or can we just add

         __attribute__((__aligned__(1 << CPU_TLB_ENTRY_BITS)))

to the struct?  I'm not sure if it affects the sizeof too, so that
requires some care.  Alternatively, an

        uint8_t padding[0]
                __attribute__((__aligned__(1 << CPU_TLB_ENTRY_BITS)));

could maybe work?  Neither is exactly the same, as they also bump the
alignment of the overall struct, but they do not require anonymous structs.

Paolo

> +        /* padding to get a power of two size */
> +        uint8_t dummy[1 << CPU_TLB_ENTRY_BITS];
> +    };
>  } CPUTLBEntry;
>  
>  QEMU_BUILD_BUG_ON(sizeof(CPUTLBEntry) != (1 << CPU_TLB_ENTRY_BITS));
>