Re: [Qemu-devel] [RFC qom-cpu 03/41] cpu: Turn cpu_get_tb_cpu_state() into a CPUClass hook

[Andreas Färber]

Am 04.09.2013 12:26, schrieb Paolo Bonzini:
> Il 04/09/2013 11:04, Andreas Färber ha scritto:
>>  static inline TranslationBlock *tb_find_fast(CPUArchState *env)
>>  {
>> +    CPUState *cpu = ENV_GET_CPU(env);
>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>>      TranslationBlock *tb;
>> -    target_ulong cs_base, pc;
>> +    vaddr cs_base, pc;
>>      int flags;
>>  
>>      /* we record a subset of the CPU state. It will
>>         always be the same before a given translated block
>>         is executed. */
>> -    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
>> +    cc->get_tb_cpu_state(cpu, &pc, &cs_base, &flags);
> 
> I'm afraid you cannot turn inline functions into indirect calls like
> this in hot paths.
> 
> One alternative would be to hoist the function call to the beginning of
> cpu_exec, and ensure that any place that modifies the result calls
> cpu_exit.  It may be a problem for SPARC's npc stuff, for which I have
> no idea how it works.

Sorry, you lost me here...

> Another is to change cpu-exec.c into a file that is #included by
> target-*/helper.c or something like that.  This way cpu-exec.c can still
> use the inline functions.

I don't see how that would help with compiling multiple CPU types into
one executable. A common CPU struct type is needed to compile the core
CPU code once, which in turn requires dispatching for target-specific
bits, such as this one or previously gdbstub and TBD monitor.

Combining only targets with target_ulong of the same size and identical
endianness is a restriction we can accept, I think - examples include
32-bit ARM+SH4A, ARM+MicroBlaze, ARM+Hexagon, ARM+Epiphany.

For performance reasons I have been careful not to have an, e.g.,
cpu_get_tb_cpu_state() wrapper that calls CPU_GET_CLASS() each time.
Many of the "cpu" variables added are being cleaned up later in the
series by argument propagation. And in placement of variables requiring
CPU() cast I have been careful to place them depending on where they
are/will be actually used rather than always placing them at the top.
But if behavior depends on the CPU type, then it cannot be a global
function - cpu.h as-is is a problem and needs to be broken up.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg