[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 02/11] tcg: light re-factor and pass down Tra
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v4 02/11] tcg: light re-factor and pass down TranslationBlock |
Date: |
Tue, 4 Aug 2015 14:36:28 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On 2015-08-03 10:14, Alex Bennée wrote:
> My later debugging patches need access to the origin PC. At the same
> time we have a slightly clumsy pass-by-reference access to the size of
> the translated block again for debugging purposes.
>
> To simplify the code I have expanded the TranslationBlock structure to
> include a tc_size variable to compliment the tc_ptr (and the subject pc,
> block size). This is set on code generation and then accessed directly
> by all the people that need it.
>
> I've also cleaned up some comments and removed un-used return variables.
>
> Signed-off-by: Alex Bennée <address@hidden>
>
> ---
> v1
> - checkpatch fixes
> ---
> include/exec/exec-all.h | 4 ++--
> tcg/tcg.c | 22 +++++++++++++---------
> tcg/tcg.h | 5 ++---
> translate-all.c | 43 ++++++++++++++++++-------------------------
> 4 files changed, 35 insertions(+), 39 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index a6fce04..7ac8e7e 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -78,8 +78,7 @@ void restore_state_to_opc(CPUArchState *env, struct
> TranslationBlock *tb,
> int pc_pos);
>
> void cpu_gen_init(void);
> -int cpu_gen_code(CPUArchState *env, struct TranslationBlock *tb,
> - int *gen_code_size_ptr);
> +void cpu_gen_code(CPUArchState *env, struct TranslationBlock *tb);
> bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc);
> void page_size_init(void);
>
> @@ -153,6 +152,7 @@ struct TranslationBlock {
> #define CF_USE_ICOUNT 0x20000
>
> void *tc_ptr; /* pointer to the translated code */
> + uint32_t tc_size;/* size of translated code */
> /* next matching tb for physical address. */
> struct TranslationBlock *phys_hash_next;
> /* first and second physical page containing code. The lower bit
What's the impact on the memory here? Given the alignement, we add 8
bytes to the structure on a 64-bit host.
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 0892a9b..587bd89 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -2295,12 +2295,15 @@ void tcg_dump_op_count(FILE *f, fprintf_function
> cpu_fprintf)
> #endif
>
>
> -static inline int tcg_gen_code_common(TCGContext *s,
> - tcg_insn_unit *gen_code_buf,
> +static inline int tcg_gen_code_common(TCGContext *s, TranslationBlock *tb,
> long search_pc)
> {
> int oi, oi_next;
>
> + /* if we are coming via cpu_restore_state we already have a
> + generated block */
> + g_assert(tb->tc_size == 0 || search_pc > 0);
In TCG code, you should use tcg_debug_assert.
> +
> #ifdef DEBUG_DISAS
> if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP))) {
> qemu_log("OP:\n");
> @@ -2338,8 +2341,8 @@ static inline int tcg_gen_code_common(TCGContext *s,
>
> tcg_reg_alloc_start(s);
>
> - s->code_buf = gen_code_buf;
> - s->code_ptr = gen_code_buf;
> + s->code_buf = tb->tc_ptr;
> + s->code_ptr = tb->tc_ptr;
>
> tcg_out_tb_init(s);
>
> @@ -2402,7 +2405,7 @@ static inline int tcg_gen_code_common(TCGContext *s,
> return -1;
> }
>
> -int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
> +void tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> {
> #ifdef CONFIG_PROFILER
> {
> @@ -2422,22 +2425,23 @@ int tcg_gen_code(TCGContext *s, tcg_insn_unit
> *gen_code_buf)
> }
> #endif
>
> - tcg_gen_code_common(s, gen_code_buf, -1);
> + tcg_gen_code_common(s, tb, -1);
>
> /* flush instruction cache */
> flush_icache_range((uintptr_t)s->code_buf, (uintptr_t)s->code_ptr);
>
> - return tcg_current_code_size(s);
> + tb->tc_size = tcg_current_code_size(s);
> + return;
> }
>
> /* Return the index of the micro operation such as the pc after is <
> offset bytes from the start of the TB. The contents of gen_code_buf must
> not be changed, though writing the same values is ok.
> Return -1 if not found. */
> -int tcg_gen_code_search_pc(TCGContext *s, tcg_insn_unit *gen_code_buf,
> +int tcg_gen_code_search_pc(TCGContext *s, TranslationBlock *tb,
> long offset)
> {
> - return tcg_gen_code_common(s, gen_code_buf, offset);
> + return tcg_gen_code_common(s, tb, offset);
> }
>
> #ifdef CONFIG_PROFILER
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 231a781..e4f9f97 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -613,9 +613,8 @@ void tcg_context_init(TCGContext *s);
> void tcg_prologue_init(TCGContext *s);
> void tcg_func_start(TCGContext *s);
>
> -int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf);
> -int tcg_gen_code_search_pc(TCGContext *s, tcg_insn_unit *gen_code_buf,
> - long offset);
> +void tcg_gen_code(TCGContext *s, TranslationBlock *tb);
> +int tcg_gen_code_search_pc(TCGContext *s, TranslationBlock *tb, long
> offset);
>
> void tcg_set_frame(TCGContext *s, int reg, intptr_t start, intptr_t size);
>
> diff --git a/translate-all.c b/translate-all.c
> index c05e2a5..e8072d8 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -157,17 +157,12 @@ void cpu_gen_init(void)
> tcg_context_init(&tcg_ctx);
> }
>
> -/* return non zero if the very first instruction is invalid so that
> - the virtual CPU can trigger an exception.
> -
> - '*gen_code_size_ptr' contains the size of the generated code (host
> - code).
> +/* code generation. On return tb->tc_size will reflect the size of
> + * generated code.
> */
> -int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, int
> *gen_code_size_ptr)
> +void cpu_gen_code(CPUArchState *env, TranslationBlock *tb)
> {
> TCGContext *s = &tcg_ctx;
> - tcg_insn_unit *gen_code_buf;
> - int gen_code_size;
> #ifdef CONFIG_PROFILER
> int64_t ti;
> #endif
> @@ -184,7 +179,6 @@ int cpu_gen_code(CPUArchState *env, TranslationBlock *tb,
> int *gen_code_size_ptr
> trace_translate_block(tb, tb->pc, tb->tc_ptr);
>
> /* generate machine code */
> - gen_code_buf = tb->tc_ptr;
> tb->tb_next_offset[0] = 0xffff;
> tb->tb_next_offset[1] = 0xffff;
> s->tb_next_offset = tb->tb_next_offset;
> @@ -201,24 +195,23 @@ int cpu_gen_code(CPUArchState *env, TranslationBlock
> *tb, int *gen_code_size_ptr
> s->interm_time += profile_getclock() - ti;
> s->code_time -= profile_getclock();
> #endif
> - gen_code_size = tcg_gen_code(s, gen_code_buf);
> - *gen_code_size_ptr = gen_code_size;
> + tcg_gen_code(s, tb);
Watch the indentation here.
> +
> #ifdef CONFIG_PROFILER
> s->code_time += profile_getclock();
> s->code_in_len += tb->size;
> - s->code_out_len += gen_code_size;
> + s->code_out_len += tb->tc_size;
> #endif
>
> - tb_write_perfmap(gen_code_buf, gen_code_size, tb->pc);
> + tb_write_perfmap(tb->tc_ptr, tb->tc_size, tb->pc);
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
> - qemu_log("OUT: [size=%d]\n", gen_code_size);
> - log_disas(tb->tc_ptr, gen_code_size);
> + qemu_log("OUT: [size=%d]\n", tb->tc_size);
> + log_disas(tb->tc_ptr, tb->tc_size);
> qemu_log("\n");
> qemu_log_flush();
> }
> #endif
> - return 0;
> }
>
> /* The cpu state corresponding to 'searched_pc' is restored.
> @@ -229,7 +222,6 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
> TranslationBlock *tb,
> CPUArchState *env = cpu->env_ptr;
> TCGContext *s = &tcg_ctx;
> int j;
> - uintptr_t tc_ptr;
> #ifdef CONFIG_PROFILER
> int64_t ti;
> #endif
> @@ -249,9 +241,9 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
> TranslationBlock *tb,
> }
>
> /* find opc index corresponding to search_pc */
> - tc_ptr = (uintptr_t)tb->tc_ptr;
> - if (searched_pc < tc_ptr)
> + if (searched_pc < (uintptr_t) tb->tc_ptr) {
> return -1;
> + }
>
> s->tb_next_offset = tb->tb_next_offset;
> #ifdef USE_DIRECT_JUMP
> @@ -261,8 +253,8 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
> TranslationBlock *tb,
> s->tb_jmp_offset = NULL;
> s->tb_next = tb->tb_next;
> #endif
> - j = tcg_gen_code_search_pc(s, (tcg_insn_unit *)tc_ptr,
> - searched_pc - tc_ptr);
> + j = tcg_gen_code_search_pc(s, tb,
> + searched_pc - (uintptr_t) tb->tc_ptr);
> if (j < 0)
> return -1;
> /* now find start of instruction before */
> @@ -1029,7 +1021,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> TranslationBlock *tb;
> tb_page_addr_t phys_pc, phys_page2;
> target_ulong virt_page2;
> - int code_gen_size;
>
> phys_pc = get_page_addr_code(env, pc);
> if (use_icount) {
> @@ -1045,12 +1036,14 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> tcg_ctx.tb_ctx.tb_invalidated_flag = 1;
> }
> tb->tc_ptr = tcg_ctx.code_gen_ptr;
> + tb->tc_size = 0;
> tb->cs_base = cs_base;
> tb->flags = flags;
> tb->cflags = cflags;
> - cpu_gen_code(env, tb, &code_gen_size);
> - tcg_ctx.code_gen_ptr = (void *)(((uintptr_t)tcg_ctx.code_gen_ptr +
> - code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
> + cpu_gen_code(env, tb);
> + tcg_ctx.code_gen_ptr = (void *) (
> + ((uintptr_t) tcg_ctx.code_gen_ptr + tb->tc_size + CODE_GEN_ALIGN - 1)
> + & ~(CODE_GEN_ALIGN - 1));
>
> /* check next page if needed */
> virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
> --
> 2.5.0
>
>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
address@hidden http://www.aurel32.net