From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: vandersonmr <vandersonmr2@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH v6 09/10] monitor: adding new info cfg command
Date: Thu, 22 Aug 2019 11:00:44 +0100 [thread overview]
Message-ID: <20190822100044.GE3277@work-vm> (raw)
In-Reply-To: <20190821172329.2062-10-vandersonmr2@gmail.com>
* vandersonmr (vandersonmr2@gmail.com) wrote:
> Adding "info cfg id depth" commands to HMP.
> This command allow the exploration a TB
> neighbors by dumping [and opening] a .dot
> file with the TB CFG neighbors colorized
> by their hotness.
>
> The goal of this command is to allow the dynamic exploration
> of TCG behavior and code quality. Therefore, for now, a
> corresponding QMP command is not worthwhile.
>
> Signed-off-by: Vanderson M. do Rosario <vandersonmr2@gmail.com>
> ---
> accel/tcg/tb-stats.c | 169 ++++++++++++++++++++++++++++++++++++++++
> hmp-commands-info.hx | 7 ++
> include/exec/tb-stats.h | 1 +
> monitor/misc.c | 22 ++++++
> 4 files changed, 199 insertions(+)
>
> diff --git a/accel/tcg/tb-stats.c b/accel/tcg/tb-stats.c
> index 875bc026b7..5bdcad2d80 100644
> --- a/accel/tcg/tb-stats.c
> +++ b/accel/tcg/tb-stats.c
> @@ -657,6 +657,174 @@ void dump_tb_info(int id, int log_mask, bool use_monitor)
> /* tbdi free'd by do_dump_tb_info_safe */
> }
>
> +/* TB CFG xdot/dot dump implementation */
> +#define MAX_CFG_NUM_NODES 1000
> +static int cfg_tb_id;
> +static GHashTable *cfg_nodes;
> +static uint64_t root_count;
> +
> +static void fputs_jump(TBStatistics *from, TBStatistics *to, FILE *dot)
> +{
> + if (!from || !to) {
> + return;
> + }
> +
> + int *from_id = (int *) g_hash_table_lookup(cfg_nodes, from);
> + int *to_id = (int *) g_hash_table_lookup(cfg_nodes, to);
> +
> + if (!from_id || !to_id) {
> + return;
> + }
> +
> + GString *line = g_string_new(NULL);
> +
> + g_string_printf(line, " node_%d -> node_%d;\n", *from_id, *to_id);
> +
> + fputs(line->str, dot);
> +
> + g_string_free(line, true);
Why not just:
fprintf(dot, " node_%d -> node_%d;\n", *from_id, *to_id);
and similar below.
> +}
> +
> +static void fputs_tbstats(TBStatistics *tbs, FILE *dot, int log_flags)
> +{
> + if (!tbs) {
> + return;
> + }
> +
> + GString *line = g_string_new(NULL);;
> +
> + uint32_t color = 0xFF666;
> + uint64_t count = tbs->executions.normal;
> + if (count > 1.6 * root_count) {
> + color = 0xFF000;
> + } else if (count > 1.2 * root_count) {
> + color = 0xFF333;
> + } else if (count < 0.4 * root_count) {
> + color = 0xFFCCC;
> + } else if (count < 0.8 * root_count) {
> + color = 0xFF999;
> + }
Lots of magical constants with no comments; preferably put constants in
separate defines and comment them.
> +
> + GString *code_s = get_code_string(tbs, log_flags);
> +
> + for (int i = 0; i < code_s->len; i++) {
> + if (code_s->str[i] == '\n') {
> + code_s->str[i] = ' ';
> + code_s = g_string_insert(code_s, i, "\\l");
> + i += 2;
> + }
> + }
> +
> + g_string_printf(line,
> + " node_%d [fillcolor=\"#%xFF0000\" shape=\"record\" "
> + "label=\"TB %d\\l"
> + "-------------\\l"
> + "PC:\t0x"TARGET_FMT_lx"\\l"
> + "exec count:\t%lu\\l"
> + "\\l %s\"];\n",
> + cfg_tb_id, color, cfg_tb_id, tbs->pc,
> + tbs->executions.normal, code_s->str);
> +
> + fputs(line->str, dot);
> +
> + int *id = g_new(int, 1);
> + *id = cfg_tb_id;
> + g_hash_table_insert(cfg_nodes, tbs, id);
> +
> + cfg_tb_id++;
> +
> + g_string_free(line, true);
> + g_string_free(code_s, true);
> +}
> +
> +static void fputs_preorder_walk(TBStatistics *tbs, int depth, FILE *dot, int log_flags)
> +{
> + if (tbs && depth > 0
> + && cfg_tb_id < MAX_CFG_NUM_NODES
> + && !g_hash_table_contains(cfg_nodes, tbs)) {
> +
> + fputs_tbstats(tbs, dot, log_flags);
> +
> + if (tbs->tb) {
> + TranslationBlock *left_tb = NULL;
> + TranslationBlock *right_tb = NULL;
> + if (tbs->tb->jmp_dest[0]) {
> + left_tb = (TranslationBlock *) atomic_read(tbs->tb->jmp_dest);
> + }
> + if (tbs->tb->jmp_dest[1]) {
> + right_tb = (TranslationBlock *) atomic_read(tbs->tb->jmp_dest + 1);
> + }
> +
> + if (left_tb) {
> + fputs_preorder_walk(left_tb->tb_stats, depth - 1, dot, log_flags);
> + fputs_jump(tbs, left_tb->tb_stats, dot);
> + }
> + if (right_tb) {
> + fputs_preorder_walk(right_tb->tb_stats, depth - 1, dot, log_flags);
> + fputs_jump(tbs, right_tb->tb_stats, dot);
> + }
> + }
> + }
> +}
> +
> +struct PreorderInfo {
> + TBStatistics *tbs;
> + int depth;
> + int log_flags;
> +};
> +
> +static void fputs_preorder_walk_safe(CPUState *cpu, run_on_cpu_data icmd)
> +{
> + struct PreorderInfo *info = icmd.host_ptr;
> +
> + GString *file_name = g_string_new(NULL);;
> + g_string_printf(file_name, "/tmp/qemu-cfg-tb-%d-%d.dot", id, info->depth);
> + FILE *dot = fopen(file_name->str, "w+");
> +
> + fputs(
> + "digraph G {\n"
> + " mclimit=1.5;\n"
> + " rankdir=TD; ordering=out;\n"
> + " graph[fontsize=10 fontname=\"Verdana\"];\n"
> + " color=\"#efefef\";\n"
> + " node[shape=box style=filled fontsize=8 fontname=\"Verdana\" fillcolor=\"#efefef\"];\n"
> + " edge[fontsize=8 fontname=\"Verdana\"];\n"
> + , dot);
> +
> + cfg_nodes = g_hash_table_new(NULL, NULL);
> + fputs_preorder_walk(info->tbs, info->depth, dot, info->log_flags);
> + g_hash_table_destroy(cfg_nodes);
> +
> + fputs("}\n\0", dot);
> + fclose(dot);
> +
> + qemu_log("CFG dumped: %s\n", file_name->str);
> +
> + g_string_free(file_name, true);
> + g_free(info);
> +}
> +
> +void dump_tb_cfg(int id, int depth, int log_flags)
> +{
> + cfg_tb_id = 1;
> + root_count = 0;
> +
> + /* do a pre-order walk in the CFG with a limited depth */
> + TBStatistics *root = get_tbstats_by_id(id);
> + if (root) {
> + root_count = root->executions.normal;
> + }
> +
> + struct PreorderInfo *info = g_new(struct PreorderInfo, 1);
> + info->tbs = root;
> + info->depth = depth + 1;
> + info->log_flags = log_flags;
> + async_safe_run_on_cpu(first_cpu, fputs_preorder_walk_safe,
> + RUN_ON_CPU_HOST_PTR(info));
> +}
> +
> +/* TBStatistic collection controls */
> +
>
> void enable_collect_tb_stats(void)
> {
> @@ -706,3 +874,4 @@ uint32_t get_default_tbstats_flag(void)
> {
> return default_tbstats_flag;
> }
> +
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index f415479011..8c96924c0b 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -305,6 +305,13 @@ ETEXI
> "dump flags can be used to set dump code level: out_asm in_asm op",
> .cmd = hmp_info_tb,
> },
> + {
> + .name = "cfg",
> + .args_type = "id:i,depth:i?,flags:s?",
> + .params = "id [depth flags]",
> + .help = "plot CFG around TB with the given id",
> + .cmd = hmp_info_cfg,
> + },
> {
> .name = "coverset",
> .args_type = "coverage:i?",
> diff --git a/include/exec/tb-stats.h b/include/exec/tb-stats.h
> index 69c3a8e5e1..0d7a03bdfa 100644
> --- a/include/exec/tb-stats.h
> +++ b/include/exec/tb-stats.h
> @@ -129,6 +129,7 @@ void dump_tbs_info(int count, int sort_by, bool use_monitor);
> */
> void dump_tb_info(int id, int log_mask, bool use_monitor);
>
> +void dump_tb_cfg(int id, int depth, int log_flags);
>
> /* TBStatistic collection controls */
> void enable_collect_tb_stats(void);
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 26b19c62dc..5cce807bb9 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -556,6 +556,28 @@ static void hmp_info_tb(Monitor *mon, const QDict *qdict)
> dump_tb_info(id, mask, true);
> }
>
> +static void hmp_info_cfg(Monitor *mon, const QDict *qdict)
> +{
> + const int id = qdict_get_int(qdict, "id");
> + const int depth = qdict_get_try_int(qdict, "depth", 3);
> + const char *flags = qdict_get_try_str(qdict, "flags");
> + int mask;
> +
> + if (!tcg_enabled()) {
> + error_report("TB information is only available with accel=tcg");
> + return;
> + }
> +
> + mask = flags ? qemu_str_to_log_mask(flags) : CPU_LOG_TB_IN_ASM;
> +
> + if (!mask) {
> + error_report("Unable to parse log flags, see 'help log'");
> + return;
> + }
> +
> + dump_tb_cfg(id, depth, mask);
> +}
> +
> static void hmp_info_coverset(Monitor *mon, const QDict *qdict)
> {
> int coverage;
> --
> 2.22.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2019-08-22 10:03 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-21 17:23 [Qemu-devel] [PATCH v6 00/10] Measure Tiny Code Generation Quality vandersonmr
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 01/10] accel: introducing TBStatistics structure vandersonmr
2019-08-26 16:24 ` Alex Bennée
2019-08-26 16:25 ` Alex Bennée
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 02/10] accel: collecting TB execution count vandersonmr
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 03/10] accel: collecting JIT statistics vandersonmr
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 04/10] accel: replacing part of CONFIG_PROFILER with TBStats vandersonmr
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 05/10] accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER vandersonmr
2019-08-27 13:11 ` Alex Bennée
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 06/10] log: adding -d tb_stats to control tbstats vandersonmr
2019-08-27 13:31 ` Alex Bennée
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 07/10] monitor: adding tb_stats hmp command vandersonmr
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 08/10] Adding info [tb-list|tb|coverset] commands to HMP vandersonmr
2019-08-22 9:52 ` Dr. David Alan Gilbert
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 09/10] monitor: adding new info cfg command vandersonmr
2019-08-22 10:00 ` Dr. David Alan Gilbert [this message]
2019-08-21 17:23 ` [Qemu-devel] [PATCH v6 10/10] linux-user: dumping hot TBs at the end of the execution vandersonmr
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=20190822100044.GE3277@work-vm \
--to=dgilbert@redhat.com \
--cc=armbru@redhat.com \
--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).