Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Laurent Desnogues]

On Wed, Apr 6, 2016 at 7:42 PM, Richard Henderson <address@hidden> wrote:
> On 04/06/2016 10:32 AM, Emilio G. Cota wrote:
>> On Wed, Apr 06, 2016 at 08:06:57 +0200, Laurent Desnogues wrote:
>>> On Tue, Apr 5, 2016 at 7:19 PM, Richard Henderson <address@hidden> wrote:
>>>> On 04/05/2016 09:33 AM, Laurent Desnogues wrote:
>>>>> The 'flags' field is 64-bit.  You're thinking of cflags, I guess.
>>>>
>>>> Well that's silly.  Since it's filled in via
>>>>
>>>> static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong 
>>>> *pc,
>>>>                                         target_ulong *cs_base, int *flags)
>>>>
>>>> and passed back in to generate code with
>>>>
>>>> TranslationBlock *tb_gen_code(CPUState *cpu,
>>>>                               target_ulong pc, target_ulong cs_base, int 
>>>> flags,
>>>>                               int cflags);
>>>>
>>>> So while TranslationBlock stores "uint64_t", the producer and consumer see 
>>>> "int".
>>>
>>> I agree.  I guess TranslationBlock should be fixed to use uint32_t
>>> (note several functions have to be changed from using int to uint32_t
>>> or aarch64-softmmu will fail).
>>
>> Can you please elaborate on this?
>
> The arm port is using some high bits, including
>
> #define ARM_TBFLAG_AARCH64_STATE_SHIFT 31
> #define ARM_TBFLAG_AARCH64_STATE_MASK  (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT)

Yes, that's why I advocate the use of uint32_t over int.

Emilio, I can't reproduce the failure I had, but fixing the warnings
gcc issues when using uint32_t instead of int for cpu_get_tb_cpu_state
should be enough to make the change safe.  Note this doesn't really
belong to this patch set :-)


Laurent