Vanderson Martins do Rosario <vanderson...@gmail.com> writes:
> Ok. I haven't change it before because I would like to be able to collect > information for already translated TBs when I, for instance, remove the > filter during execution. Having the TBStats already created guarantee this. > To guarantee this in your approach, we would need to tb_flush when changing > the filter. Does it make sense? Is that ok for you? I think so. While tb_flush is a bit of hammer translation is pretty cheap so things will be running pretty quickly afterwards. We don't need to flush the old TB stats entries though - we can keep them for the lifetime of the run. > > Vanderson M. Rosario > > > On Fri, Aug 30, 2019 at 7:21 AM Alex Bennée <alex.ben...@linaro.org> wrote: > >> >> vandersonmr <vanderson...@gmail.com> writes: >> >> > If a TB has a TBS (TBStatistics) with the TB_EXEC_STATS >> > enabled, then we instrument the start code of this TB >> > to atomically count the number of times it is executed. >> > We count both the number of "normal" executions and atomic >> > executions of a TB. >> > >> > The execution count of the TB is stored in its respective >> > TBS. >> > >> > All TBStatistics are created by default with the flags from >> > default_tbstats_flag. >> > >> > Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com> >> > --- >> > accel/tcg/cpu-exec.c | 4 ++++ >> > accel/tcg/tb-stats.c | 5 +++++ >> > accel/tcg/tcg-runtime.c | 7 +++++++ >> > accel/tcg/tcg-runtime.h | 2 ++ >> > accel/tcg/translate-all.c | 7 +++++++ >> > accel/tcg/translator.c | 1 + >> > include/exec/gen-icount.h | 9 +++++++++ >> > include/exec/tb-stats.h | 19 +++++++++++++++++++ >> > util/log.c | 1 + >> > 9 files changed, 55 insertions(+) >> > >> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> > index 48272c781b..9b2b7bff80 100644 >> > --- a/accel/tcg/cpu-exec.c >> > +++ b/accel/tcg/cpu-exec.c >> > @@ -251,6 +251,10 @@ void cpu_exec_step_atomic(CPUState *cpu) >> > >> > start_exclusive(); >> > >> > + if (tb_stats_enabled(tb, TB_EXEC_STATS)) { >> > + tb->tb_stats->executions.atomic++; >> > + } >> > + >> > /* Since we got here, we know that parallel_cpus must be true. >> */ >> > parallel_cpus = false; >> > in_exclusive_region = true; >> > diff --git a/accel/tcg/tb-stats.c b/accel/tcg/tb-stats.c >> > index 948b107e68..1db81d83e7 100644 >> > --- a/accel/tcg/tb-stats.c >> > +++ b/accel/tcg/tb-stats.c >> > @@ -61,3 +61,8 @@ bool tb_stats_collection_paused(void) >> > { >> > return tcg_collect_tb_stats == TB_STATS_PAUSED; >> > } >> > + >> > +uint32_t get_default_tbstats_flag(void) >> > +{ >> > + return default_tbstats_flag; >> > +} >> > diff --git a/accel/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c >> > index 8a1e408e31..6f4aafba11 100644 >> > --- a/accel/tcg/tcg-runtime.c >> > +++ b/accel/tcg/tcg-runtime.c >> > @@ -167,3 +167,10 @@ void HELPER(exit_atomic)(CPUArchState *env) >> > { >> > cpu_loop_exit_atomic(env_cpu(env), GETPC()); >> > } >> > + >> > +void HELPER(inc_exec_freq)(void *ptr) >> > +{ >> > + TBStatistics *stats = (TBStatistics *) ptr; >> > + g_assert(stats); >> > + atomic_inc(&stats->executions.normal); >> > +} >> > 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/accel/tcg/translate-all.c b/accel/tcg/translate-all.c >> > index b7bccacd3b..e72aeba682 100644 >> > --- a/accel/tcg/translate-all.c >> > +++ b/accel/tcg/translate-all.c >> > @@ -1785,6 +1785,13 @@ TranslationBlock *tb_gen_code(CPUState *cpu, >> > */ >> > if (tb_stats_collection_enabled()) { >> > tb->tb_stats = tb_get_stats(phys_pc, pc, cs_base, flags, tb); >> > + >> > + if (qemu_log_in_addr_range(tb->pc)) { >> >> We can open this out because this test will always pass if no dfilter >> has been set and there is no point creating a tb_stats record if we >> won't fill it in. So >> >> if (qemu_log_in_addr_range(tb->pc)) { >> tb->tb_stats = tb_get_stats(phys_pc, pc, cs_base, flags, tb); >> uint32_t flag = get_default_tbstats_flag(); >> >> if (flag & TB_EXEC_STATS) { >> ... >> >> And the additional tests that get added later. This way we'll only >> create and collect stats for what we want. >> >> > + uint32_t flag = get_default_tbstats_flag(); >> > + if (flag & TB_EXEC_STATS) { >> > + tb->tb_stats->stats_enabled |= TB_EXEC_STATS; >> > + } >> > + } >> > } else { >> > tb->tb_stats = NULL; >> > } >> > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c >> > index 70c66c538c..ec6bd829a0 100644 >> > --- a/accel/tcg/translator.c >> > +++ b/accel/tcg/translator.c >> > @@ -46,6 +46,7 @@ void translator_loop(const TranslatorOps *ops, >> DisasContextBase *db, >> > >> > ops->init_disas_context(db, cpu); >> > tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */ >> > + gen_tb_exec_count(tb); >> > >> > /* Reset the temp count so that we can identify leaks */ >> > tcg_clear_temp_count(); >> > diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h >> > index 822c43cfd3..be006383b9 100644 >> > --- a/include/exec/gen-icount.h >> > +++ b/include/exec/gen-icount.h >> > @@ -32,6 +32,15 @@ static inline void gen_io_end(void) >> > tcg_temp_free_i32(tmp); >> > } >> > >> > +static inline void gen_tb_exec_count(TranslationBlock *tb) >> > +{ >> > + if (tb_stats_enabled(tb, TB_EXEC_STATS)) { >> > + TCGv_ptr ptr = tcg_const_ptr(tb->tb_stats); >> > + gen_helper_inc_exec_freq(ptr); >> > + tcg_temp_free_ptr(ptr); >> > + } >> > +} >> > + >> > static inline void gen_tb_start(TranslationBlock *tb) >> > { >> > TCGv_i32 count, imm; >> > diff --git a/include/exec/tb-stats.h b/include/exec/tb-stats.h >> > index 898e05a36f..c4a8715400 100644 >> > --- a/include/exec/tb-stats.h >> > +++ b/include/exec/tb-stats.h >> > @@ -30,6 +30,9 @@ >> > #include "exec/tb-context.h" >> > #include "tcg.h" >> > >> > +#define tb_stats_enabled(tb, JIT_STATS) \ >> > + (tb && tb->tb_stats && (tb->tb_stats->stats_enabled & JIT_STATS)) >> > + >> > typedef struct TBStatistics TBStatistics; >> > >> > /* >> > @@ -46,6 +49,15 @@ struct TBStatistics { >> > uint32_t flags; >> > /* cs_base isn't included in the hash but we do check for matches */ >> > target_ulong cs_base; >> > + >> > + uint32_t stats_enabled; >> > + >> > + /* Execution stats */ >> > + struct { >> > + unsigned long normal; >> > + unsigned long atomic; >> > + } executions; >> > + >> > /* current TB linked to this TBStatistics */ >> > TranslationBlock *tb; >> > }; >> > @@ -56,7 +68,12 @@ void init_tb_stats_htable_if_not(void); >> > >> > /* TBStatistic collection controls */ >> > enum TBStatsStatus { TB_STATS_RUNNING, TB_STATS_PAUSED, >> TB_STATS_STOPPED }; >> > + >> > +#define TB_NOTHING (1 << 0) >> > +#define TB_EXEC_STATS (1 << 1) >> > + >> > extern int tcg_collect_tb_stats; >> > +extern uint32_t default_tbstats_flag; >> > >> > void enable_collect_tb_stats(void); >> > void disable_collect_tb_stats(void); >> > @@ -64,4 +81,6 @@ void pause_collect_tb_stats(void); >> > bool tb_stats_collection_enabled(void); >> > bool tb_stats_collection_paused(void); >> > >> > +uint32_t get_default_tbstats_flag(void); >> > + >> > #endif >> > diff --git a/util/log.c b/util/log.c >> > index 393a17115b..29021a4584 100644 >> > --- a/util/log.c >> > +++ b/util/log.c >> > @@ -32,6 +32,7 @@ static int log_append = 0; >> > static GArray *debug_regions; >> > >> > int tcg_collect_tb_stats; >> > +uint32_t default_tbstats_flag; >> > >> > /* Return the number of characters emitted. */ >> > int qemu_log(const char *fmt, ...) >> >> >> -- >> Alex Bennée >> -- Alex Bennée