On 08/09/2016 14:44, Alex Bennée wrote: >> > cpu->tb_flushed = false; /* reset before first TB lookup */ >> > for(;;) { >> > cpu_handle_interrupt(cpu, &last_tb); >> > - tb = tb_find_fast(cpu, &last_tb, tb_exit); >> > + tb = tb_find_fast(cpu, last_tb, tb_exit); > Maybe a comment here for those that missed the subtly in the commit > message? > > /* cpu_loop_exec_tb updates a to a new last_tb */ > >> > cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc); > You could even make it explicit and change cpu_loop_exec_tb to return > last_tb instead of passing by reference. Then it would be even clearer > when reading the code. >
I gave it a quick shot and it's not that simple... One simpler possibility is to take this patch one step further and merge "tb" and "last_tb", but I've not tested it yet: diff --git a/cpu-exec.c b/cpu-exec.c index cf511f1..80e6ff5 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -515,11 +515,11 @@ static inline void cpu_handle_interrupt(CPUState *cpu, } } -static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb, - TranslationBlock **last_tb, int *tb_exit, - SyncClocks *sc) +static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock **last_tb, + int *tb_exit, SyncClocks *sc) { uintptr_t ret; + TranslationBlock *tb = *last_tb; if (unlikely(cpu->exit_request)) { return; @@ -609,7 +609,7 @@ int cpu_exec(CPUState *cpu) for(;;) { /* prepare setjmp context for exception handling */ if (sigsetjmp(cpu->jmp_env, 0) == 0) { - TranslationBlock *tb, *last_tb = NULL; + TranslationBlock *tb = NULL; int tb_exit = 0; /* if an exception is pending, we execute it here */ @@ -619,9 +619,9 @@ int cpu_exec(CPUState *cpu) cpu->tb_flushed = false; /* reset before first TB lookup */ for(;;) { - cpu_handle_interrupt(cpu, &last_tb); - tb = tb_find_fast(cpu, last_tb, tb_exit); - cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc); + cpu_handle_interrupt(cpu, &tb); + tb = tb_find_fast(cpu, tb, tb_exit); + cpu_loop_exec_tb(cpu, &tb, &tb_exit, &sc); /* Try to align the host and virtual clocks if the guest is in advance */ align_clocks(&sc, cpu); It seems better to me to do it as a follow-up step. Paolo