Message ID | 20190614135332.12777-2-vandersonmr2@gmail.com |
---|---|
State | New |
Headers | show |
Series | Collecting TB Execution Frequency | expand |
On 6/14/19 6:53 AM, vandersonmr wrote: > +void HELPER(inc_exec_freq)(void *ptr) > +{ > + TranslationBlock* tb = (TranslationBlock*) ptr; > + atomic_inc(&tb->exec_freq); > +} ... > +DEF_HELPER_FLAGS_1(inc_exec_freq, TCG_CALL_NO_RWG, void, ptr) ... > uint32_t flags; /* flags defining in which context the code was generated */ > uint16_t size; /* size of target code for this block (1 <= > size <= TARGET_PAGE_SIZE) */ > + uint64_t exec_freq; It's not a frequency, but a count. > uint16_t icount; > uint32_t cflags; /* compile flags */ Consider where you've placed the data with respect to the packing of other members of the structure. > static inline void gen_tb_start(TranslationBlock *tb) > { > TCGv_i32 count, imm; > > + if (enable_freq_count) { > + TCGv_ptr tb_ptr = tcg_temp_new_ptr(); > + tcg_gen_trunc_i64_ptr(tb_ptr, tcg_const_i64((int64_t) tb)); > + gen_helper_inc_exec_freq(tb_ptr); > + } > + > tcg_ctx->exitreq_label = gen_new_label(); > if (tb_cflags(tb) & CF_USE_ICOUNT) { > count = tcg_temp_local_new_i32(); By placing the increment before the exit for interrupt check instead of after, you're kinda counting the wrong thing, because the TB hasn't executed. > diff --git a/linux-user/main.c b/linux-user/main.c > index a59ae9439d..1bf7155670 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -58,6 +58,7 @@ static const char *seed_optarg; > unsigned long mmap_min_addr; > unsigned long guest_base; > int have_guest_base; > +bool enable_freq_count = false; This is being declared in multiple places and initialized in multiple places. This needs to go elsewhere. r~
[repeat of reply to wrong email...] vandersonmr <vandersonmr2@gmail.com> writes: > An uint64_t counter was added in the TranslationBlock struct and > it is incremented every time that the TB is executed. > > Signed-off-by: vandersonmr <vandersonmr2@gmail.com> > --- > accel/tcg/tcg-runtime.c | 6 ++++++ > accel/tcg/tcg-runtime.h | 2 ++ > include/exec/exec-all.h | 1 + > include/exec/gen-icount.h | 7 +++++++ > linux-user/main.c | 1 + > vl.c | 1 + > 6 files changed, 18 insertions(+) > > diff --git a/accel/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c > index 8a1e408e31..d1f4127d31 100644 > --- a/accel/tcg/tcg-runtime.c > +++ b/accel/tcg/tcg-runtime.c > @@ -167,3 +167,9 @@ void HELPER(exit_atomic)(CPUArchState *env) > { > cpu_loop_exit_atomic(env_cpu(env), GETPC()); > } > + > +void HELPER(inc_exec_freq)(void *ptr) > +{ > + TranslationBlock* tb = (TranslationBlock*) ptr; > + atomic_inc(&tb->exec_freq); > +} > diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h > index 4fa61b49b4..bf0b75dbe8 100644 > --- a/accel/tcg/tcg-runtime.h > +++ b/accel/tcg/tcg-runtime.h > @@ -28,6 +28,8 @@ DEF_HELPER_FLAGS_1(lookup_tb_ptr, TCG_CALL_NO_WG_SE, ptr, env) > > DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env) > > +DEF_HELPER_FLAGS_1(inc_exec_freq, TCG_CALL_NO_RWG, void, ptr) > + > #ifdef CONFIG_SOFTMMU > > DEF_HELPER_FLAGS_5(atomic_cmpxchgb, TCG_CALL_NO_WG, > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 16034ee651..a80407622e 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -342,6 +342,7 @@ struct TranslationBlock { > uint32_t flags; /* flags defining in which context the code was generated */ > uint16_t size; /* size of target code for this block (1 <= > size <= TARGET_PAGE_SIZE) */ > + uint64_t exec_freq; Maybe exec_count would be more correct. Frequency occurs over a given time. Also a single line comment would be useful. > uint16_t icount; > uint32_t cflags; /* compile flags */ > #define CF_COUNT_MASK 0x00007fff > diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h > index f7669b6841..a540d12005 100644 > --- a/include/exec/gen-icount.h > +++ b/include/exec/gen-icount.h > @@ -6,11 +6,18 @@ > /* Helpers for instruction counting code generation. */ > > static TCGOp *icount_start_insn; > +extern bool enable_freq_count; > > static inline void gen_tb_start(TranslationBlock *tb) > { > TCGv_i32 count, imm; > > + if (enable_freq_count) { > + TCGv_ptr tb_ptr = tcg_temp_new_ptr(); > + tcg_gen_trunc_i64_ptr(tb_ptr, tcg_const_i64((int64_t) tb)); TIL: tcg_const_ptr which elides the details of casting to a TCGv_ptr on 32 and 64 bit hosts. So: TCGv_ptr tb_ptr = tcg_const_ptr(tb); gen_helper_inc_exec_freq(tb_ptr); tcg_temp_free_ptr(tb_ptr); (don't forget to free temporaries once used, otherwise the translator will keep tracking them. This should show up in a --enable-debug-tcg build). > + gen_helper_inc_exec_freq(tb_ptr); > + } > + I think we should move the counter bellow the icount check otherwise we might be counting TB's that never execute because an IRQ was pending. > tcg_ctx->exitreq_label = gen_new_label(); > if (tb_cflags(tb) & CF_USE_ICOUNT) { > count = tcg_temp_local_new_i32(); > diff --git a/linux-user/main.c b/linux-user/main.c > index a59ae9439d..1bf7155670 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -58,6 +58,7 @@ static const char *seed_optarg; > unsigned long mmap_min_addr; > unsigned long guest_base; > int have_guest_base; > +bool enable_freq_count = false; > > /* > * When running 32-on-64 we should make sure we can fit all of the possible > diff --git a/vl.c b/vl.c > index 005468cbfb..cb6c0ad63d 100644 > --- a/vl.c > +++ b/vl.c > @@ -190,6 +190,7 @@ bool boot_strict; > uint8_t *boot_splash_filedata; > int only_migratable; /* turn it off unless user states otherwise */ > bool wakeup_suspend_enabled; > +bool enable_freq_count = false; We only need the space allocated once in common code. Maybe accel/tcg/translator.c? Also you don't need to initialise to false (== 0 == initial bss setting). > > int icount_align_option; -- Alex Bennée
diff --git a/accel/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c index 8a1e408e31..d1f4127d31 100644 --- a/accel/tcg/tcg-runtime.c +++ b/accel/tcg/tcg-runtime.c @@ -167,3 +167,9 @@ void HELPER(exit_atomic)(CPUArchState *env) { cpu_loop_exit_atomic(env_cpu(env), GETPC()); } + +void HELPER(inc_exec_freq)(void *ptr) +{ + TranslationBlock* tb = (TranslationBlock*) ptr; + atomic_inc(&tb->exec_freq); +} diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h index 4fa61b49b4..bf0b75dbe8 100644 --- a/accel/tcg/tcg-runtime.h +++ b/accel/tcg/tcg-runtime.h @@ -28,6 +28,8 @@ DEF_HELPER_FLAGS_1(lookup_tb_ptr, TCG_CALL_NO_WG_SE, ptr, env) DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env) +DEF_HELPER_FLAGS_1(inc_exec_freq, TCG_CALL_NO_RWG, void, ptr) + #ifdef CONFIG_SOFTMMU DEF_HELPER_FLAGS_5(atomic_cmpxchgb, TCG_CALL_NO_WG, diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 16034ee651..a80407622e 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -342,6 +342,7 @@ struct TranslationBlock { uint32_t flags; /* flags defining in which context the code was generated */ uint16_t size; /* size of target code for this block (1 <= size <= TARGET_PAGE_SIZE) */ + uint64_t exec_freq; uint16_t icount; uint32_t cflags; /* compile flags */ #define CF_COUNT_MASK 0x00007fff diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index f7669b6841..a540d12005 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -6,11 +6,18 @@ /* Helpers for instruction counting code generation. */ static TCGOp *icount_start_insn; +extern bool enable_freq_count; static inline void gen_tb_start(TranslationBlock *tb) { TCGv_i32 count, imm; + if (enable_freq_count) { + TCGv_ptr tb_ptr = tcg_temp_new_ptr(); + tcg_gen_trunc_i64_ptr(tb_ptr, tcg_const_i64((int64_t) tb)); + gen_helper_inc_exec_freq(tb_ptr); + } + tcg_ctx->exitreq_label = gen_new_label(); if (tb_cflags(tb) & CF_USE_ICOUNT) { count = tcg_temp_local_new_i32(); diff --git a/linux-user/main.c b/linux-user/main.c index a59ae9439d..1bf7155670 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -58,6 +58,7 @@ static const char *seed_optarg; unsigned long mmap_min_addr; unsigned long guest_base; int have_guest_base; +bool enable_freq_count = false; /* * When running 32-on-64 we should make sure we can fit all of the possible diff --git a/vl.c b/vl.c index 005468cbfb..cb6c0ad63d 100644 --- a/vl.c +++ b/vl.c @@ -190,6 +190,7 @@ bool boot_strict; uint8_t *boot_splash_filedata; int only_migratable; /* turn it off unless user states otherwise */ bool wakeup_suspend_enabled; +bool enable_freq_count = false; int icount_align_option;
An uint64_t counter was added in the TranslationBlock struct and it is incremented every time that the TB is executed. Signed-off-by: vandersonmr <vandersonmr2@gmail.com> --- accel/tcg/tcg-runtime.c | 6 ++++++ accel/tcg/tcg-runtime.h | 2 ++ include/exec/exec-all.h | 1 + include/exec/gen-icount.h | 7 +++++++ linux-user/main.c | 1 + vl.c | 1 + 6 files changed, 18 insertions(+)