Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

[Peter Crosthwaite]

On Mon, Apr 27, 2015 at 12:00 PM, Peter Maydell
<address@hidden> wrote:
> On 27 April 2015 at 13:58, Peter Maydell <address@hidden> wrote:
>> The tcg/tcg.h comment should be updated:
>>  * Otherwise, we gave up on execution of this TB before it started, and
>>  * the caller must fix up the CPU state by calling the CPU's
>>  * synchronize_from_tb() method with the next-TB pointer we return.
>>  * (falling back to calling the CPU's set_pc() method with tb->pc
>>  * if no synchronize_from_tb() method exists.)
>>
>> That's a bit clunky though, which suggests we should
>> have a cpu_synchronize_from_tb() inline function in qom/cpu.h
>> which does the
>>         CPUClass *cc = CPU_GET_CLASS(cpu);
>>         if (cc->synchronize_from_tb) {
>>             cc->synchronize_from_tb(cpu, tb);
>>         } else {
>>             assert(cc->set_pc);
>>             cc->set_pc(cpu, tb->pc);
>>         }
>>
>> bit that cpu-exec.c currently open-codes.
>
> ...except that qom/cpu.h doesn't have the definition of the
> TranslationBlock struct it would need to be able to do that "tb->pc".
>  * we can't just include exec-all.h from here or otherwise get
>    the TranslationBlock struct definition, because it is target
>    CPU dependent
>  * we can't have the common baseclass in qom/cpu.c provide an
>    implementation of the synchronize_from_tb method which calls
>    set_pc, because qom/cpu.c is a common-obj-y sourcefile]
> which leaves us with:
>  * have cpu_synchronize_from_tb() take both tb and tb->pc as args,
>    which is pretty yucky
>  * give up and just update the tcg.h comment as above
>
> I think I go for "just update the comment"...
>

Does that mean a respin of this patch or should the tricore fix and
the comment be a follow up?

Regards,
Peter

> -- PMM
>