From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
vandersonmr <vandersonmr2@gmail.com>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH v5 02/10] accel: collecting TB execution count
Date: Thu, 15 Aug 2019 14:38:39 +0100 [thread overview]
Message-ID: <87r25mmtuo.fsf@linaro.org> (raw)
In-Reply-To: <20190815021857.19526-3-vandersonmr2@gmail.com>
vandersonmr <vandersonmr2@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 <vandersonmr2@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 6c85c3ee1e..e54be69499 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -252,6 +252,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 02844717cb..3489133e9e 100644
> --- a/accel/tcg/tb-stats.c
> +++ b/accel/tcg/tb-stats.c
> @@ -37,3 +37,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..df08d183df 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);
> + uint32_t flag = get_default_tbstats_flag();
> +
> + if (qemu_log_in_addr_range(tb->pc)) {
Minor nit: the compiler should spot it but you could move the flag
fetching inside this block.
> + 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 9226a348a3..396a11e828 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 f7669b6841..b3efe41894 100644
> --- a/include/exec/gen-icount.h
> +++ b/include/exec/gen-icount.h
> @@ -7,6 +7,15 @@
>
> static TCGOp *icount_start_insn;
>
> +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 cc8f8a6ce6..0265050b79 100644
> --- a/include/exec/tb-stats.h
> +++ b/include/exec/tb-stats.h
> @@ -6,6 +6,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;
>
> /*
> @@ -22,6 +25,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;
> };
> @@ -32,7 +44,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 0
> +#define TB_EXEC_STATS 1
As this is going to be a bit field you should use:
#define TB_NOTHING (1 << 0)
etc...
> +
> extern int tcg_collect_tb_stats;
> +extern uint32_t default_tbstats_flag;
>
> void enable_collect_tb_stats(void);
> void disable_collect_tb_stats(void);
> @@ -40,4 +57,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;
This should static and in tb_stats.c (like the helper is)
>
> /* Return the number of characters emitted. */
> int qemu_log(const char *fmt, ...)
--
Alex Bennée
next prev parent reply other threads:[~2019-08-15 13:39 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-15 2:18 [Qemu-devel] [PATCH v5 00/10] Measure Tiny Code Generation Quality vandersonmr
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 01/10] accel: introducing TBStatistics structure vandersonmr
2019-08-15 13:13 ` Alex Bennée
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 02/10] accel: collecting TB execution count vandersonmr
2019-08-15 13:38 ` Alex Bennée [this message]
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 03/10] accel: collecting JIT statistics vandersonmr
2019-08-15 14:29 ` Alex Bennée
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 04/10] accel: replacing part of CONFIG_PROFILER with TBStats vandersonmr
2019-08-15 14:54 ` Alex Bennée
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 05/10] accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER vandersonmr
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 06/10] log: adding -d tb_stats to control tbstats vandersonmr
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 07/10] monitor: adding tb_stats hmp command vandersonmr
2019-08-15 8:53 ` Dr. David Alan Gilbert
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 08/10] Adding info [tbs|tb|coverset] commands to HMP. These commands allow the exploration of TBs generated by the TCG. Understand which one hotter, with more guest/host instructions... and examine their guest, host and IR code vandersonmr
2019-08-15 8:59 ` Dr. David Alan Gilbert
2019-08-21 14:16 ` Vanderson Martins do Rosario
2019-08-21 14:29 ` Dr. David Alan Gilbert
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 09/10] monitor: adding new info cfg command vandersonmr
2019-08-15 9:14 ` Dr. David Alan Gilbert
2019-08-15 2:18 ` [Qemu-devel] [PATCH v5 10/10] linux-user: dumping hot TBs at the end of the execution vandersonmr
2019-08-15 14:26 ` Aleksandar Markovic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r25mmtuo.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
--cc=vandersonmr2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).