On 10/04/2012 08:15 PM, Stefan Weil wrote: > Am 04.10.2012 12:36, schrieb Avi Kivity: >> The hassle and compile time overhead of maintaining both 32-bit and >> 64-bit >> capable source isn't worth the tiny performance advantage which is >> seen on >> a minority of configurations. Switch to compiling libhw only once, with >> target_phys_addr_t unconditionally typedefed to uint64_t. >> >> Signed-off-by: Avi Kivity <a...@redhat.com> > > Hi, > > I noticed that you replaced target_phys_addr_t by uint64_t in two lines.
In those two lines, int64_t is more correct than t_p_a_t. Explanation below. > Are there plans to replace target_phys_addr_t everywhere? Yes, by hw_addr (or hwaddr, or phys, or ...). > Should new code use uint64_t, or should it continue to use > target_phys_addr_t? target_phys_addr_t. > > Using target_phys_addr_t might make support for 128 bit in some years > easier > because it allows identifying critical code, Agree. > although I think it will be difficult to avoid wrong use of either > target_phys_addr_t > or uint64_t as long as both are the same size. Some languages allow enforcing this, but C doesn't. > > >> -#if TARGET_PHYS_ADDR_BITS > 32 >> - return low | ((target_phys_addr_t)high << 32); >> -#else >> - return low; >> -#endif >> + return low | ((uint64_t)high << 32); >> Shifting by 32 is not defined for types that are 32 bits or narrower. On x86, for example, a shift by 32 of a 32-bit quantity is the identity operation (where mathematically you would expect the result to be 0, not the first argument). Since the context does not guarantee that target_phys_addr_t is wider than 32 bits, I cast it to a known-wide type, then (implicitly) cast it back to target_phys_addr_t. > >> - *raddrp |= (target_phys_addr_t)(tlb->RPN & 0xF) << 32; >> + *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32; > Same applies here, of course. Since target_phys_addr_t is 64 bits, it would have worked "by accident", but if we'd have switched back to 32 bits in the future (unlikely but possible) then I would have introduced a bug here. -- error compiling committee.c: too many arguments to function
