Re: [Qemu-devel] [PATCH v4 00/26] Do away with TB retranslation

[Aurelien Jarno]

On 2015-09-30 15:09, Richard Henderson wrote:
> Version 4:
>   * Typos noticed by pmm during round 3.
>   * RB's collected during round 3.
>   * Reduce the number of TBs allocated.
> 
> I believe the only patch left without an RB is 24; and of
> course 26 which is new.
> 
> 
> r~
> 
> 
> Richard Henderson (26):
>   tcg: Rename debug_insn_start to insn_start
>   target-*: Unconditionally emit tcg_gen_insn_start
>   target-*: Increment num_insns immediately after tcg_gen_insn_start
>   target-*: Introduce and use cpu_breakpoint_test
>   tcg: Allow extra data to be attached to insn_start
>   target-arm: Add condexec state to insn_start
>   target-i386: Add cc_op state to insn_start
>   target-mips: Add delayed branch state to insn_start
>   target-s390x: Add cc_op state to insn_start
>   target-sh4: Add flags state to insn_start
>   target-cris: Mirror gen_opc_pc into insn_start
>   target-sparc: Tidy gen_branch_a interface
>   target-sparc: Split out gen_branch_n
>   target-sparc: Remove gen_opc_jump_pc
>   target-sparc: Add npc state to insn_start
>   tcg: Merge cpu_gen_code into tb_gen_code
>   target-*: Drop cpu_gen_code define
>   tcg: Add TCG_MAX_INSNS
>   tcg: Pass data argument to restore_state_to_opc
>   tcg: Save insn data and use it in cpu_restore_state_from_tb
>   tcg: Remove gen_intermediate_code_pc
>   tcg: Remove tcg_gen_code_search_pc
>   tcg: Emit prologue to the beginning of code_gen_buffer
>   tcg: Allocate a guard page after code_gen_buffer
>   tcg: Check for overflow via highwater mark
>   tcg: Adjust CODE_GEN_AVG_BLOCK_SIZE
> 
>  include/exec/exec-all.h       |  23 +-
>  include/qom/cpu.h             |  16 ++
>  target-alpha/cpu.h            |   1 -
>  target-alpha/translate.c      |  70 ++----
>  target-arm/cpu.h              |   2 +-
>  target-arm/translate-a64.c    |  48 +---
>  target-arm/translate.c        |  83 +++----
>  target-arm/translate.h        |   8 +-
>  target-cris/cpu.h             |   1 -
>  target-cris/translate.c       |  93 ++------
>  target-cris/translate_v10.c   |   3 -
>  target-i386/cpu.h             |   2 +-
>  target-i386/translate.c       | 106 +++------
>  target-lm32/cpu.h             |   1 -
>  target-lm32/translate.c       |  83 ++-----
>  target-m68k/cpu.h             |   1 -
>  target-m68k/translate.c       |  82 ++-----
>  target-microblaze/cpu.h       |   1 -
>  target-microblaze/translate.c |  83 ++-----
>  target-mips/cpu.h             |   2 +-
>  target-mips/translate.c       |  98 +++-----
>  target-moxie/cpu.h            |   1 -
>  target-moxie/translate.c      |  82 +++----
>  target-openrisc/cpu.h         |   1 -
>  target-openrisc/translate.c   |  78 ++-----
>  target-ppc/cpu.h              |   1 -
>  target-ppc/translate.c        |  72 ++----
>  target-s390x/cpu.h            |   2 +-
>  target-s390x/translate.c      |  78 ++-----
>  target-sh4/cpu.h              |   2 +-
>  target-sh4/translate.c        |  91 +++-----
>  target-sparc/cpu.h            |   2 +-
>  target-sparc/translate.c      | 185 +++++++--------
>  target-tilegx/cpu.h           |   1 -
>  target-tilegx/translate.c     |  58 ++---
>  target-tricore/translate.c    |  59 ++---
>  target-unicore32/translate.c  |  83 ++-----
>  target-xtensa/cpu.h           |   1 -
>  target-xtensa/translate.c     |  79 ++-----
>  tcg/tcg-op.h                  |  52 ++++-
>  tcg/tcg-opc.h                 |   4 +-
>  tcg/tcg.c                     | 168 ++++++++------
>  tcg/tcg.h                     |  20 +-
>  tci.c                         |   9 -
>  translate-all.c               | 520 
> +++++++++++++++++++++++++-----------------
>  45 files changed, 964 insertions(+), 1492 deletions(-)

I have finally been able to review this whole patchset, sorry for the 
time it took. Thanks a lot for working on that, I think it's really a 
great improvement, both in term of performances, and in code cleanup
(look at the final diff above). It's probably possible to do some more 
code cleaning after it's applied, but that should wait until this
patchset is applied. There is at least the "do not change the jump
target while retranslating" code in a few TCG targets that can be
removed.

As said while reviewing this patchset, it opens some place for more
improvements (like not saving data for instructions which can't trigger
exceptions), but that should come after it has been applied.

On this subject, I believe this patchset is now a good state, except
maybe for the last few patches. I am running an earlier version of it
on plenty of VM without any issue, except on SH4. I realized SH4
broken in some rare cases, but not directly by this patchset. There is 
an issue when a delay slot is split in two parts when reaching the
maximum number of TCG ops. Given the patch 2 adds more TCG ops, it
triggers more often. It is already possible to trigger this issue, so 
this should not be consider as a blocker. I'll work on a fix ASAP,
which might still takes some time given my current load.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net