Re: [Qemu-devel] [PATCH 08/39] provide portable sizeof(long) test

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/12/2010 04:41 PM, malc wrote:
> > > >  Gives wrong results on Win64.
> >  
> >  Then it's not HOST_LONG_BITS, it's HOST_POINTER_BITS.
> 
> Not quite, [s]size_t/ptrdiff_t are 64 bits wide udner Win64, little
> to do with pointers.

Before (on Win64): sizeof(long) == 4, HOST_LONG_BITS == 64

After: sizeof(long) == 4, HOST_LONG_BITS == 32

If you say HOST_LONG_BITS should be 64, then I say it is poorly named: it
represents sizeof(void*) * CHAR_BIT and should be named HOST_POINTER_BITS.

BTW, HOST_LONG_BITS is used mostly for user-mode emulation, which does not
matter for Win64.  In exec-all.h it is used to second-guess
sizeof(tcg_target_long), which would be the size of a pointer.  However,
it is safe to give a conservative value based on sizeof(long).

Finally, in other places it is definitely used to mean sizeof(long).  Even
though the softmmu is probably not IL32P64-clean, there you have this:

#if HOST_LONG_BITS == 32 && TARGET_LONG_BITS == 32
#define CPU_TLB_ENTRY_BITS 4
#else
#define CPU_TLB_ENTRY_BITS 5
#endif

typedef struct CPUTLBEntry {
    /* bit TARGET_LONG_BITS to TARGET_PAGE_BITS : virtual address
       bit TARGET_PAGE_BITS-1..4  : Nonzero for accesses that should not
                                    go directly to ram.
       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.  */
    unsigned long 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(unsigned long) - 1)) + 
                   sizeof(unsigned long))];
} CPUTLBEntry;

_As things are now_ it is more correct to use sizeof(long), or at
least more conservative.

I can certainly change it to sizeof(void*) in v2, even though I don't think
it's a good idea, but it's your call as a maintainer.

Paolo