| Message ID | 1361362828-19589-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 02/20/2013 04:20 AM, Peter Maydell wrote: > Document tcg_qemu_tb_exec(). In particular, its return value is a > combination of a pointer to the next translation block and some > extra information in the low two bits. Provide some #defines for > the values passed in these bits to improve code clarity. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <rth@twiddle.net> r~
Am 20.02.2013 13:20, schrieb Peter Maydell: > Document tcg_qemu_tb_exec(). In particular, its return value is a > combination of a pointer to the next translation block and some > extra information in the low two bits. Provide some #defines for > the values passed in these bits to improve code clarity. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I have a patch cooking which uses the final remaining bottom-two-bits > combo to indicate "exited TB due to pending interrupt" so I thought it > would be nice to document what was going on here and get rid of some > of the magic numbers in the code. > > cpu-exec.c | 9 +++++---- > include/exec/gen-icount.h | 2 +- > tcg/tcg.h | 36 +++++++++++++++++++++++++++++++++++- > 3 files changed, 41 insertions(+), 6 deletions(-) > > diff --git a/cpu-exec.c b/cpu-exec.c > index 9fcfe9e0..ea63e7d 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -72,7 +72,7 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles, > next_tb = tcg_qemu_tb_exec(env, tb->tc_ptr); > cpu->current_tb = NULL; > > - if ((next_tb & 3) == 2) { > + if ((next_tb & TB_EXIT_MASK) == TB_EXIT_ICOUNT_EXPIRED) { > /* Restore PC. This may happen if async event occurs before > the TB starts executing. */ > cpu_pc_from_tb(env, tb); > @@ -584,7 +584,8 @@ int cpu_exec(CPUArchState *env) > spans two pages, we cannot safely do a direct > jump. */ > if (next_tb != 0 && tb->page_addr[1] == -1) { > - tb_add_jump((TranslationBlock *)(next_tb & ~3), next_tb & 3, tb); > + tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK), > + next_tb & TB_EXIT_MASK, tb); > } > spin_unlock(&tcg_ctx.tb_ctx.tb_lock); > > @@ -598,10 +599,10 @@ int cpu_exec(CPUArchState *env) > tc_ptr = tb->tc_ptr; > /* execute the generated code */ > next_tb = tcg_qemu_tb_exec(env, tc_ptr); > - if ((next_tb & 3) == 2) { > + if ((next_tb & TB_EXIT_MASK) == TB_EXIT_ICOUNT_EXPIRED) { > /* Instruction counter expired. */ > int insns_left; > - tb = (TranslationBlock *)(next_tb & ~3); > + tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK); > /* Restore PC. */ > cpu_pc_from_tb(env, tb); > insns_left = env->icount_decr.u32; > diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h > index 8043b3b..c858a73 100644 > --- a/include/exec/gen-icount.h > +++ b/include/exec/gen-icount.h > @@ -32,7 +32,7 @@ static void gen_icount_end(TranslationBlock *tb, int num_insns) > if (use_icount) { > *icount_arg = num_insns; > gen_set_label(icount_label); > - tcg_gen_exit_tb((tcg_target_long)tb + 2); > + tcg_gen_exit_tb((tcg_target_long)tb + TB_EXIT_ICOUNT_EXPIRED); > } > } > > diff --git a/tcg/tcg.h b/tcg/tcg.h > index 51c8176..7cf4c15 100644 > --- a/tcg/tcg.h > +++ b/tcg/tcg.h > @@ -669,7 +669,41 @@ TCGv_i64 tcg_const_i64(int64_t val); > TCGv_i32 tcg_const_local_i32(int32_t val); > TCGv_i64 tcg_const_local_i64(int64_t val); > > -/* TCG targets may use a different definition of tcg_qemu_tb_exec. */ > +/** > + * tcg_qemu_tb_exec: > + * @env: CPUArchState * for the CPU "@env: #CPUArchState for the CPU."? > + * @tb_ptr: address of generated code for the TB to execute > + * > + * Start executing code from a given translation block. > + * Where translation blocks have been linked, execution > + * may proceed from the given TB into successive ones. > + * Control eventually returns only when some action is needed > + * from the top-level loop: either control must pass to a TB > + * which has not yet been directly linked, or an asynchronous > + * event such as an interrupt needs handling. > + * > + * The return value is a pointer to the next TB to execute > + * (if known; otherwise zero). This pointer is assumed to be > + * 4-aligned, and the bottom two bits are used to return further > + * information: > + * 0, 1: the link between this TB and the next is via the specified > + * TB index (0 or 1). That is, we left the TB via (the equivalent > + * of) "goto_tb <index>". The main loop uses this to determine > + * how to link the TB just executed to the next. > + * 2: we are using instruction counting code generation, and we > + * stopped executing this TB because the instruction counter > + * hit zero. In this case the next-TB pointer returned is the > + * TB we were partway through. > + * > + * Note that TCG targets may use a different definition of tcg_qemu_tb_exec > + * to this default (which just calls the prologue code emitted by > + * tcg_target_qemu_prologue()). > + */ Are you sure it works to separate this comment from the macro by the #defines below? > +#define TB_EXIT_MASK 3 > +#define TB_EXIT_IDX0 0 > +#define TB_EXIT_IDX1 1 > +#define TB_EXIT_ICOUNT_EXPIRED 2 This smells like an enum (which would then get its own documentation comment some day). IDX0 and IDX1 don't seem to be used, do you have a follow-up? Andreas > + > #if !defined(tcg_qemu_tb_exec) > # define tcg_qemu_tb_exec(env, tb_ptr) \ > ((tcg_target_ulong (*)(void *, void *))tcg_ctx.code_gen_prologue)(env, \
On 20 February 2013 18:49, Andreas Färber <afaerber@suse.de> wrote: > Am 20.02.2013 13:20, schrieb Peter Maydell: > Are you sure it works to separate this comment from the macro by the > #defines below? My main use case for doc comments is "read them in situ in the source code" so that's all I test :-) >> +#define TB_EXIT_MASK 3 > >> +#define TB_EXIT_IDX0 0 >> +#define TB_EXIT_IDX1 1 >> +#define TB_EXIT_ICOUNT_EXPIRED 2 > > This smells like an enum (which would then get its own documentation > comment some day). IDX0 and IDX1 don't seem to be used, do you have a > follow-up? No, they're mostly provided for completeness. (The indexes go in via tcg_gen_goto_tb() calls mostly, and the TCG spec says the argument is 0 or 1; they're not mixed in with the other oddities like expiring icount at that point.) -- PMM
diff --git a/cpu-exec.c b/cpu-exec.c index 9fcfe9e0..ea63e7d 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -72,7 +72,7 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles, next_tb = tcg_qemu_tb_exec(env, tb->tc_ptr); cpu->current_tb = NULL; - if ((next_tb & 3) == 2) { + if ((next_tb & TB_EXIT_MASK) == TB_EXIT_ICOUNT_EXPIRED) { /* Restore PC. This may happen if async event occurs before the TB starts executing. */ cpu_pc_from_tb(env, tb); @@ -584,7 +584,8 @@ int cpu_exec(CPUArchState *env) spans two pages, we cannot safely do a direct jump. */ if (next_tb != 0 && tb->page_addr[1] == -1) { - tb_add_jump((TranslationBlock *)(next_tb & ~3), next_tb & 3, tb); + tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK), + next_tb & TB_EXIT_MASK, tb); } spin_unlock(&tcg_ctx.tb_ctx.tb_lock); @@ -598,10 +599,10 @@ int cpu_exec(CPUArchState *env) tc_ptr = tb->tc_ptr; /* execute the generated code */ next_tb = tcg_qemu_tb_exec(env, tc_ptr); - if ((next_tb & 3) == 2) { + if ((next_tb & TB_EXIT_MASK) == TB_EXIT_ICOUNT_EXPIRED) { /* Instruction counter expired. */ int insns_left; - tb = (TranslationBlock *)(next_tb & ~3); + tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK); /* Restore PC. */ cpu_pc_from_tb(env, tb); insns_left = env->icount_decr.u32; diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 8043b3b..c858a73 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -32,7 +32,7 @@ static void gen_icount_end(TranslationBlock *tb, int num_insns) if (use_icount) { *icount_arg = num_insns; gen_set_label(icount_label); - tcg_gen_exit_tb((tcg_target_long)tb + 2); + tcg_gen_exit_tb((tcg_target_long)tb + TB_EXIT_ICOUNT_EXPIRED); } } diff --git a/tcg/tcg.h b/tcg/tcg.h index 51c8176..7cf4c15 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -669,7 +669,41 @@ TCGv_i64 tcg_const_i64(int64_t val); TCGv_i32 tcg_const_local_i32(int32_t val); TCGv_i64 tcg_const_local_i64(int64_t val); -/* TCG targets may use a different definition of tcg_qemu_tb_exec. */ +/** + * tcg_qemu_tb_exec: + * @env: CPUArchState * for the CPU + * @tb_ptr: address of generated code for the TB to execute + * + * Start executing code from a given translation block. + * Where translation blocks have been linked, execution + * may proceed from the given TB into successive ones. + * Control eventually returns only when some action is needed + * from the top-level loop: either control must pass to a TB + * which has not yet been directly linked, or an asynchronous + * event such as an interrupt needs handling. + * + * The return value is a pointer to the next TB to execute + * (if known; otherwise zero). This pointer is assumed to be + * 4-aligned, and the bottom two bits are used to return further + * information: + * 0, 1: the link between this TB and the next is via the specified + * TB index (0 or 1). That is, we left the TB via (the equivalent + * of) "goto_tb <index>". The main loop uses this to determine + * how to link the TB just executed to the next. + * 2: we are using instruction counting code generation, and we + * stopped executing this TB because the instruction counter + * hit zero. In this case the next-TB pointer returned is the + * TB we were partway through. + * + * Note that TCG targets may use a different definition of tcg_qemu_tb_exec + * to this default (which just calls the prologue code emitted by + * tcg_target_qemu_prologue()). + */ +#define TB_EXIT_MASK 3 +#define TB_EXIT_IDX0 0 +#define TB_EXIT_IDX1 1 +#define TB_EXIT_ICOUNT_EXPIRED 2 + #if !defined(tcg_qemu_tb_exec) # define tcg_qemu_tb_exec(env, tb_ptr) \ ((tcg_target_ulong (*)(void *, void *))tcg_ctx.code_gen_prologue)(env, \
Document tcg_qemu_tb_exec(). In particular, its return value is a combination of a pointer to the next translation block and some extra information in the low two bits. Provide some #defines for the values passed in these bits to improve code clarity. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I have a patch cooking which uses the final remaining bottom-two-bits combo to indicate "exited TB due to pending interrupt" so I thought it would be nice to document what was going on here and get rid of some of the magic numbers in the code. cpu-exec.c | 9 +++++---- include/exec/gen-icount.h | 2 +- tcg/tcg.h | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 6 deletions(-)