Re: [Qemu-devel] [PATCH v2 13/45] tcg: consolidate TB lookups in tb_lookup__cpu_state

[Richard Henderson]

On 07/16/2017 10:03 AM, Emilio G. Cota wrote:
> +    if (likely(tb &&
> +               tb->pc == *pc &&
> +               tb->cs_base == *cs_base &&
> +               tb->flags == *flags &&
> +               tb->trace_vcpu_dstate == *cpu->trace_dstate)) {

I'll just mention something I noticed while looking at perf data for Alpha. 
It's a tiny thing, however: we should order these by likelyhood of mismatch.

Almost no targets really use cs_base.  X86 won't use it when in a 32-bit or 
64-bit flat address space.  Sparc stuffs NPC into cs_base, but you really have 
to explore the limits of branch delay slots in order to see NPC != PC + 4, so 
NPC very rarely differs when PC is equal.  Similarly for HPPA.

Which suggests checking flags before cs_base.

That said, I've also considered rewriting this without short-circuit ands 
(after verifying TB != NULL) so that the compiler is free to perform all of the 
subsequent loads early.

That said, this is correct as-is.

Reviewed-by: Richard Henderson <address@hidden>


r~