* [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
@ 2019-06-14 13:53 vandersonmr
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter vandersonmr
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: vandersonmr @ 2019-06-14 13:53 UTC (permalink / raw)
To: qemu-devel
This is the first series of patches related to the TCGCodeQuality GSoC project
More at https://wiki.qemu.org/Features/TCGCodeQuality
It adds an option to instrument TBs and collects their execution frequency.
The execution frequency is then store/accumulated in an auxiliary structure
every time a tb_flush or a read happens.
[Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
[Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
[Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
@ 2019-06-14 13:53 ` vandersonmr
2019-06-17 2:28 ` Richard Henderson
2019-06-17 10:01 ` Alex Bennée
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events vandersonmr
` (4 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: vandersonmr @ 2019-06-14 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Riku Voipio, vandersonmr, Laurent Vivier,
Richard Henderson
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;
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;
--
2.22.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter vandersonmr
@ 2019-06-14 13:53 ` vandersonmr
2019-06-17 2:52 ` Richard Henderson
2019-06-17 10:00 ` Alex Bennée
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user vandersonmr
` (3 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: vandersonmr @ 2019-06-14 13:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, vandersonmr, Richard Henderson
A new hash map was added to store the accumulated execution
frequency of the TBs even after tb_flush events. A dump
function was also added as a way to visualize these frequencies.
Signed-off-by: vandersonmr <vandersonmr2@gmail.com>
---
accel/tcg/translate-all.c | 59 +++++++++++++++++++++++++++++++++++++++
accel/tcg/translate-all.h | 2 ++
exec.c | 7 +++++
include/exec/exec-all.h | 3 ++
include/exec/tb-context.h | 9 ++++++
include/qom/cpu.h | 2 ++
6 files changed, 82 insertions(+)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 5d1e08b169..0bc670ffad 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1118,6 +1118,12 @@ static inline void code_gen_alloc(size_t tb_size)
}
}
+static bool statistics_cmp(const void* ap, const void *bp) {
+ const TBStatistics *a = ap;
+ const TBStatistics *b = bp;
+ return a->pc == b->pc && a->cs_base == b->cs_base && a->flags == b->flags;
+}
+
static bool tb_cmp(const void *ap, const void *bp)
{
const TranslationBlock *a = ap;
@@ -1137,6 +1143,7 @@ static void tb_htable_init(void)
unsigned int mode = QHT_MODE_AUTO_RESIZE;
qht_init(&tb_ctx.htable, tb_cmp, CODE_GEN_HTABLE_SIZE, mode);
+ qht_init(&tb_ctx.tb_statistics, statistics_cmp, CODE_GEN_HTABLE_SIZE, QHT_MODE_AUTO_RESIZE);
}
/* Must be called before using the QEMU cpus. 'tb_size' is the size
@@ -1228,6 +1235,53 @@ static gboolean tb_host_size_iter(gpointer key, gpointer value, gpointer data)
return false;
}
+static void do_tb_dump_exec_freq(void *p, uint32_t hash, void *userp)
+{
+#if TARGET_LONG_SIZE == 8
+ TBStatistics *tbs = p;
+ printf("%016lx\t%lu\n", tbs->pc, tbs->total_exec_freq);
+#elif TARGET_LONG_SIZE == 4
+ TBStatistics *tbs = p;
+ printf("%016x\t%lu\n", tbs->pc, tbs->total_exec_freq);
+#endif
+}
+
+void tb_dump_all_exec_freq(void)
+{
+ tb_read_exec_freq();
+ qht_iter(&tb_ctx.tb_statistics, do_tb_dump_exec_freq, NULL);
+}
+
+static void do_tb_read_exec_freq(void *p, uint32_t hash, void *userp)
+{
+ TranslationBlock *tb = p;
+ TBStatistics tbscmp;
+ tbscmp.pc = tb->pc;
+ tbscmp.cs_base = tb->cs_base;
+ tbscmp.flags = tb->flags;
+
+ TBStatistics *tbs = qht_lookup(userp, &tbscmp, hash);
+
+ uint64_t exec_freq = tb_get_and_reset_exec_freq((TranslationBlock*) p);
+
+ if (tbs) {
+ tbs->total_exec_freq += exec_freq;
+ } else {
+ void *existing;
+ tbs = malloc(sizeof(TBStatistics));
+ tbs->total_exec_freq = exec_freq;
+ tbs->pc = tb->pc;
+ tbs->cs_base = tb->cs_base;
+ tbs->flags = tb->flags;
+ qht_insert(userp, tbs, hash, &existing);
+ }
+}
+
+void tb_read_exec_freq(void)
+{
+ qht_iter(&tb_ctx.htable, do_tb_read_exec_freq, &tb_ctx.tb_statistics);
+}
+
/* flush all the translation blocks */
static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
{
@@ -1252,6 +1306,10 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
cpu_tb_jmp_cache_clear(cpu);
}
+ if (enable_freq_count) {
+ tb_read_exec_freq();
+ }
+
qht_reset_size(&tb_ctx.htable, CODE_GEN_HTABLE_SIZE);
page_flush_tb();
@@ -1723,6 +1781,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
tb->flags = flags;
tb->cflags = cflags;
tb->trace_vcpu_dstate = *cpu->trace_dstate;
+ tb->exec_freq = 0;
tcg_ctx->tb_cflags = cflags;
tb_overflow:
diff --git a/accel/tcg/translate-all.h b/accel/tcg/translate-all.h
index 64f5fd9a05..e13088c36d 100644
--- a/accel/tcg/translate-all.h
+++ b/accel/tcg/translate-all.h
@@ -32,6 +32,8 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
int is_cpu_write_access);
void tb_check_watchpoint(CPUState *cpu);
+extern bool enable_freq_count;
+
#ifdef CONFIG_USER_ONLY
int page_unprotect(target_ulong address, uintptr_t pc);
#endif
diff --git a/exec.c b/exec.c
index e7622d1956..9b64a012ee 100644
--- a/exec.c
+++ b/exec.c
@@ -1013,6 +1013,13 @@ const char *parse_cpu_option(const char *cpu_option)
return cpu_type;
}
+uint64_t tb_get_and_reset_exec_freq(TranslationBlock *tb)
+{
+ uint64_t exec_freq = atomic_load_acquire(&tb->exec_freq);
+ atomic_store_release(&tb->exec_freq, 0);
+ return exec_freq;
+}
+
#if defined(CONFIG_USER_ONLY)
void tb_invalidate_phys_addr(target_ulong addr)
{
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a80407622e..5d3d829d18 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -513,4 +513,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
/* vl.c */
extern int singlestep;
+void tb_read_exec_freq(void);
+void tb_dump_all_exec_freq(void);
+
#endif
diff --git a/include/exec/tb-context.h b/include/exec/tb-context.h
index feb585e0a7..ba518d47a0 100644
--- a/include/exec/tb-context.h
+++ b/include/exec/tb-context.h
@@ -28,6 +28,14 @@
typedef struct TranslationBlock TranslationBlock;
typedef struct TBContext TBContext;
+typedef struct TBStatistics TBStatistics;
+
+struct TBStatistics {
+ target_ulong pc;
+ target_ulong cs_base;
+ uint32_t flags;
+ uint64_t total_exec_freq;
+};
struct TBContext {
@@ -35,6 +43,7 @@ struct TBContext {
/* statistics */
unsigned tb_flush_count;
+ struct qht tb_statistics;
};
extern TBContext tb_ctx;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 5ee0046b62..593c1f1137 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -474,6 +474,8 @@ static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
}
}
+uint64_t tb_get_and_reset_exec_freq(struct TranslationBlock*);
+
/**
* qemu_tcg_mttcg_enabled:
* Check whether we are running MultiThread TCG or not.
--
2.22.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter vandersonmr
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events vandersonmr
@ 2019-06-14 13:53 ` vandersonmr
2019-06-17 2:54 ` Richard Henderson
2019-06-17 11:30 ` Alex Bennée
2019-06-14 16:01 ` [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency no-reply
` (2 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: vandersonmr @ 2019-06-14 13:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, vandersonmr, Laurent Vivier
Added -execfreq to enable execution frequency counting and dump
all the TB's addresses and their execution frequency at the end
of the execution.
Signed-off-by: vandersonmr <vandersonmr2@gmail.com>
---
linux-user/exit.c | 5 +++++
linux-user/main.c | 7 +++++++
2 files changed, 12 insertions(+)
diff --git a/linux-user/exit.c b/linux-user/exit.c
index bdda720553..0c6a2f2d5b 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -26,8 +26,13 @@
extern void __gcov_dump(void);
#endif
+extern bool enable_freq_count;
+
void preexit_cleanup(CPUArchState *env, int code)
{
+ if (enable_freq_count) {
+ tb_dump_all_exec_freq();
+ }
#ifdef TARGET_GPROF
_mcleanup();
#endif
diff --git a/linux-user/main.c b/linux-user/main.c
index 1bf7155670..ece2d8bd8b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -388,6 +388,11 @@ static void handle_arg_trace(const char *arg)
trace_file = trace_opt_parse(arg);
}
+static void handle_arg_execfreq(const char *arg)
+{
+ enable_freq_count = true;
+}
+
struct qemu_argument {
const char *argv;
const char *env;
@@ -439,6 +444,8 @@ static const struct qemu_argument arg_table[] = {
"", "Seed for pseudo-random number generator"},
{"trace", "QEMU_TRACE", true, handle_arg_trace,
"", "[[enable=]<pattern>][,events=<file>][,file=<file>]"},
+ {"execfreq", "QEMU_EXEC_FREQ", false, handle_arg_execfreq,
+ "", "enable and dump TB's execution frequency counting"},
{"version", "QEMU_VERSION", false, handle_arg_version,
"", "display version information and exit"},
{NULL, NULL, false, NULL, NULL, NULL}
--
2.22.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
` (2 preceding siblings ...)
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user vandersonmr
@ 2019-06-14 16:01 ` no-reply
2019-06-17 10:17 ` Alex Bennée
2019-06-17 13:13 ` Alex Bennée
5 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2019-06-14 16:01 UTC (permalink / raw)
To: vandersonmr2; +Cc: qemu-devel
Patchew URL: https://patchew.org/QEMU/20190614135332.12777-1-vandersonmr2@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
Type: series
Message-id: 20190614135332.12777-1-vandersonmr2@gmail.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/155912548463.2019004.3515830305299809902.stgit@bahia.lan -> patchew/155912548463.2019004.3515830305299809902.stgit@bahia.lan
- [tag update] patchew/156051774276.244890.8660277280145466396.stgit@bahia.lan -> patchew/156051774276.244890.8660277280145466396.stgit@bahia.lan
Switched to a new branch 'test'
6512387 Adding command line option to linux-user.
af52efd Saving counters between tb_flush events.
906c8ef Adding an optional tb execution counter.
=== OUTPUT BEGIN ===
1/3 Checking commit 906c8ef92aac (Adding an optional tb execution counter.)
ERROR: "(foo*)" should be "(foo *)"
#24: FILE: accel/tcg/tcg-runtime.c:173:
+ TranslationBlock* tb = (TranslationBlock*) ptr;
ERROR: do not initialise globals to 0 or NULL
#83: FILE: linux-user/main.c:61:
+bool enable_freq_count = false;
ERROR: do not initialise globals to 0 or NULL
#95: FILE: vl.c:193:
+bool enable_freq_count = false;
total: 3 errors, 0 warnings, 56 lines checked
Patch 1/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit af52efd7842d (Saving counters between tb_flush events.)
ERROR: "foo* bar" should be "foo *bar"
#22: FILE: accel/tcg/translate-all.c:1121:
+static bool statistics_cmp(const void* ap, const void *bp) {
ERROR: open brace '{' following function declarations go on the next line
#22: FILE: accel/tcg/translate-all.c:1121:
+static bool statistics_cmp(const void* ap, const void *bp) {
ERROR: line over 90 characters
#35: FILE: accel/tcg/translate-all.c:1146:
+ qht_init(&tb_ctx.tb_statistics, statistics_cmp, CODE_GEN_HTABLE_SIZE, QHT_MODE_AUTO_RESIZE);
ERROR: "(foo*)" should be "(foo *)"
#70: FILE: accel/tcg/translate-all.c:1265:
+ uint64_t exec_freq = tb_get_and_reset_exec_freq((TranslationBlock*) p);
ERROR: "(foo*)" should be "(foo *)"
#190: FILE: include/qom/cpu.h:477:
+uint64_t tb_get_and_reset_exec_freq(struct TranslationBlock*);
total: 5 errors, 0 warnings, 146 lines checked
Patch 2/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/3 Checking commit 651238793473 (Adding command line option to linux-user.)
ERROR: externs should be avoided in .c files
#22: FILE: linux-user/exit.c:29:
+extern bool enable_freq_count;
total: 1 errors, 0 warnings, 32 lines checked
Patch 3/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190614135332.12777-1-vandersonmr2@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter vandersonmr
@ 2019-06-17 2:28 ` Richard Henderson
2019-06-17 10:01 ` Alex Bennée
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2019-06-17 2:28 UTC (permalink / raw)
To: vandersonmr, qemu-devel
Cc: Paolo Bonzini, Riku Voipio, Laurent Vivier, Richard Henderson
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~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events vandersonmr
@ 2019-06-17 2:52 ` Richard Henderson
2019-06-17 10:00 ` Alex Bennée
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2019-06-17 2:52 UTC (permalink / raw)
To: vandersonmr, qemu-devel; +Cc: Paolo Bonzini, Richard Henderson
On 6/14/19 6:53 AM, vandersonmr wrote:
> A new hash map was added to store the accumulated execution
> frequency of the TBs even after tb_flush events. A dump
> function was also added as a way to visualize these frequencies.
>
> Signed-off-by: vandersonmr <vandersonmr2@gmail.com>
> ---
> accel/tcg/translate-all.c | 59 +++++++++++++++++++++++++++++++++++++++
> accel/tcg/translate-all.h | 2 ++
> exec.c | 7 +++++
> include/exec/exec-all.h | 3 ++
> include/exec/tb-context.h | 9 ++++++
> include/qom/cpu.h | 2 ++
> 6 files changed, 82 insertions(+)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 5d1e08b169..0bc670ffad 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1118,6 +1118,12 @@ static inline void code_gen_alloc(size_t tb_size)
> }
> }
>
> +static bool statistics_cmp(const void* ap, const void *bp) {
Watch the formatting.
> +static void do_tb_dump_exec_freq(void *p, uint32_t hash, void *userp)
> +{
> +#if TARGET_LONG_SIZE == 8
> + TBStatistics *tbs = p;
> + printf("%016lx\t%lu\n", tbs->pc, tbs->total_exec_freq);
> +#elif TARGET_LONG_SIZE == 4
> + TBStatistics *tbs = p;
> + printf("%016x\t%lu\n", tbs->pc, tbs->total_exec_freq);
> +#endif
> +}
TARGET_FMT_lx.
> +static void do_tb_read_exec_freq(void *p, uint32_t hash, void *userp)
> +{
> + TranslationBlock *tb = p;
> + TBStatistics tbscmp;
> + tbscmp.pc = tb->pc;
> + tbscmp.cs_base = tb->cs_base;
> + tbscmp.flags = tb->flags;
> +
> + TBStatistics *tbs = qht_lookup(userp, &tbscmp, hash);
> +
> + uint64_t exec_freq = tb_get_and_reset_exec_freq((TranslationBlock*) p);
> +
> + if (tbs) {
> + tbs->total_exec_freq += exec_freq;
> + } else {
> + void *existing;
> + tbs = malloc(sizeof(TBStatistics));
> + tbs->total_exec_freq = exec_freq;
> + tbs->pc = tb->pc;
> + tbs->cs_base = tb->cs_base;
> + tbs->flags = tb->flags;
> + qht_insert(userp, tbs, hash, &existing);
If you're going to ignore the result, leave the last argument NULL.
> + }
> +}
> +
> +void tb_read_exec_freq(void)
> +{
> + qht_iter(&tb_ctx.htable, do_tb_read_exec_freq, &tb_ctx.tb_statistics);
> +}
Perhaps a comment that this is called with mmap_lock held.
> +extern bool enable_freq_count;
Second declaration.
> +uint64_t tb_get_and_reset_exec_freq(TranslationBlock *tb)
> +{
> + uint64_t exec_freq = atomic_load_acquire(&tb->exec_freq);
> + atomic_store_release(&tb->exec_freq, 0);
> + return exec_freq;
> +}
What are you intending here? Either this needs a comment that it is called
with a lock held, and this does not need barriers at all (atomic_read,
atomic_set). Or this should use atomic_xchg and do the load and store in one
atomic operation.
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user vandersonmr
@ 2019-06-17 2:54 ` Richard Henderson
2019-06-17 11:30 ` Alex Bennée
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2019-06-17 2:54 UTC (permalink / raw)
To: vandersonmr, qemu-devel; +Cc: Riku Voipio, Laurent Vivier
On 6/14/19 6:53 AM, vandersonmr wrote:
> Added -execfreq to enable execution frequency counting and dump
> all the TB's addresses and their execution frequency at the end
> of the execution.
>
> Signed-off-by: vandersonmr <vandersonmr2@gmail.com>
> ---
> linux-user/exit.c | 5 +++++
> linux-user/main.c | 7 +++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/linux-user/exit.c b/linux-user/exit.c
> index bdda720553..0c6a2f2d5b 100644
> --- a/linux-user/exit.c
> +++ b/linux-user/exit.c
> @@ -26,8 +26,13 @@
> extern void __gcov_dump(void);
> #endif
>
> +extern bool enable_freq_count;
A third declaration.
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events vandersonmr
2019-06-17 2:52 ` Richard Henderson
@ 2019-06-17 10:00 ` Alex Bennée
1 sibling, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2019-06-17 10:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, vandersonmr, Richard Henderson
vandersonmr <vandersonmr2@gmail.com> writes:
> A new hash map was added to store the accumulated execution
> frequency of the TBs even after tb_flush events. A dump
> function was also added as a way to visualize these frequencies.
>
> Signed-off-by: vandersonmr <vandersonmr2@gmail.com>
I forgot to mention the formatting looks a little off here. It should be
your full name (accents and all) before the email address, e.g:
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> accel/tcg/translate-all.c | 59 +++++++++++++++++++++++++++++++++++++++
> accel/tcg/translate-all.h | 2 ++
> exec.c | 7 +++++
> include/exec/exec-all.h | 3 ++
> include/exec/tb-context.h | 9 ++++++
> include/qom/cpu.h | 2 ++
> 6 files changed, 82 insertions(+)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 5d1e08b169..0bc670ffad 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1118,6 +1118,12 @@ static inline void code_gen_alloc(size_t tb_size)
> }
> }
>
> +static bool statistics_cmp(const void* ap, const void *bp) {
> + const TBStatistics *a = ap;
> + const TBStatistics *b = bp;
> + return a->pc == b->pc && a->cs_base == b->cs_base && a->flags ==
> b->flags;
Looking at tb_cmp() bellow this I wonder if we should also take into
account the page_address values. Some TB's will be translated over a
page boundary and in theory that can change with new mappings so we need
to ensure they are really equivalent.
> +}
> +
> static bool tb_cmp(const void *ap, const void *bp)
> {
> const TranslationBlock *a = ap;
> @@ -1137,6 +1143,7 @@ static void tb_htable_init(void)
> unsigned int mode = QHT_MODE_AUTO_RESIZE;
>
> qht_init(&tb_ctx.htable, tb_cmp, CODE_GEN_HTABLE_SIZE, mode);
> + qht_init(&tb_ctx.tb_statistics, statistics_cmp, CODE_GEN_HTABLE_SIZE, QHT_MODE_AUTO_RESIZE);
> }
>
> /* Must be called before using the QEMU cpus. 'tb_size' is the size
> @@ -1228,6 +1235,53 @@ static gboolean tb_host_size_iter(gpointer key, gpointer value, gpointer data)
> return false;
> }
>
> +static void do_tb_dump_exec_freq(void *p, uint32_t hash, void *userp)
> +{
> +#if TARGET_LONG_SIZE == 8
> + TBStatistics *tbs = p;
> + printf("%016lx\t%lu\n", tbs->pc, tbs->total_exec_freq);
> +#elif TARGET_LONG_SIZE == 4
> + TBStatistics *tbs = p;
> + printf("%016x\t%lu\n", tbs->pc, tbs->total_exec_freq);
> +#endif
> +}
This will need some fixing up so deal with output to the HMP monitor. We
don't want to be directly spamming stdout with results.
> +
> +void tb_dump_all_exec_freq(void)
> +{
> + tb_read_exec_freq();
> + qht_iter(&tb_ctx.tb_statistics, do_tb_dump_exec_freq, NULL);
> +}
I would be tempted to insert these into a sorted GList first so we can
dump the first N hotblocks.
> +
> +static void do_tb_read_exec_freq(void *p, uint32_t hash, void *userp)
> +{
> + TranslationBlock *tb = p;
> + TBStatistics tbscmp;
> + tbscmp.pc = tb->pc;
> + tbscmp.cs_base = tb->cs_base;
> + tbscmp.flags = tb->flags;
> +
> + TBStatistics *tbs = qht_lookup(userp, &tbscmp, hash);
> +
> + uint64_t exec_freq = tb_get_and_reset_exec_freq((TranslationBlock*) p);
> +
> + if (tbs) {
> + tbs->total_exec_freq += exec_freq;
> + } else {
> + void *existing;
> + tbs = malloc(sizeof(TBStatistics));
use g_new0(TBStatistics, 1)
> + tbs->total_exec_freq = exec_freq;
> + tbs->pc = tb->pc;
> + tbs->cs_base = tb->cs_base;
> + tbs->flags = tb->flags;
> + qht_insert(userp, tbs, hash, &existing);
> + }
> +}
> +
Comment here about what we are doing? Maybe tb_copy_old_counts()?
> +void tb_read_exec_freq(void)
> +{
> + qht_iter(&tb_ctx.htable, do_tb_read_exec_freq, &tb_ctx.tb_statistics);
> +}
> +
> /* flush all the translation blocks */
> static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
> {
> @@ -1252,6 +1306,10 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
> cpu_tb_jmp_cache_clear(cpu);
> }
>
> + if (enable_freq_count) {
> + tb_read_exec_freq();
> + }
> +
> qht_reset_size(&tb_ctx.htable, CODE_GEN_HTABLE_SIZE);
> page_flush_tb();
>
> @@ -1723,6 +1781,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> tb->flags = flags;
> tb->cflags = cflags;
> tb->trace_vcpu_dstate = *cpu->trace_dstate;
> + tb->exec_freq = 0;
> tcg_ctx->tb_cflags = cflags;
> tb_overflow:
>
> diff --git a/accel/tcg/translate-all.h b/accel/tcg/translate-all.h
> index 64f5fd9a05..e13088c36d 100644
> --- a/accel/tcg/translate-all.h
> +++ b/accel/tcg/translate-all.h
> @@ -32,6 +32,8 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> int is_cpu_write_access);
> void tb_check_watchpoint(CPUState *cpu);
>
> +extern bool enable_freq_count;
> +
> #ifdef CONFIG_USER_ONLY
> int page_unprotect(target_ulong address, uintptr_t pc);
> #endif
> diff --git a/exec.c b/exec.c
> index e7622d1956..9b64a012ee 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1013,6 +1013,13 @@ const char *parse_cpu_option(const char *cpu_option)
> return cpu_type;
> }
>
> +uint64_t tb_get_and_reset_exec_freq(TranslationBlock *tb)
> +{
> + uint64_t exec_freq = atomic_load_acquire(&tb->exec_freq);
> + atomic_store_release(&tb->exec_freq, 0);
I'm not sure you need acquire/release semantics here as you are only
reading this in an exclusive period when all in-flight updates should
be synced (locks are implicit barriers). Folding this up into
do_tb_read_exec_freq might make this clearer.
> + return exec_freq;
> +}
> +
> #if defined(CONFIG_USER_ONLY)
> void tb_invalidate_phys_addr(target_ulong addr)
> {
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index a80407622e..5d3d829d18 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -513,4 +513,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> /* vl.c */
> extern int singlestep;
>
> +void tb_read_exec_freq(void);
> +void tb_dump_all_exec_freq(void);
> +
> #endif
> diff --git a/include/exec/tb-context.h b/include/exec/tb-context.h
> index feb585e0a7..ba518d47a0 100644
> --- a/include/exec/tb-context.h
> +++ b/include/exec/tb-context.h
> @@ -28,6 +28,14 @@
>
> typedef struct TranslationBlock TranslationBlock;
> typedef struct TBContext TBContext;
> +typedef struct TBStatistics TBStatistics;
> +
> +struct TBStatistics {
> + target_ulong pc;
> + target_ulong cs_base;
> + uint32_t flags;
> + uint64_t total_exec_freq;
> +};
>
> struct TBContext {
>
> @@ -35,6 +43,7 @@ struct TBContext {
>
> /* statistics */
> unsigned tb_flush_count;
> + struct qht tb_statistics;
> };
>
> extern TBContext tb_ctx;
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 5ee0046b62..593c1f1137 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -474,6 +474,8 @@ static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
> }
> }
>
> +uint64_t tb_get_and_reset_exec_freq(struct TranslationBlock*);
> +
> /**
> * qemu_tcg_mttcg_enabled:
> * Check whether we are running MultiThread TCG or not.
--
Alex Bennée
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter vandersonmr
2019-06-17 2:28 ` Richard Henderson
@ 2019-06-17 10:01 ` Alex Bennée
1 sibling, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2019-06-17 10:01 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Riku Voipio, vandersonmr, Laurent Vivier,
Richard Henderson
[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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
` (3 preceding siblings ...)
2019-06-14 16:01 ` [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency no-reply
@ 2019-06-17 10:17 ` Alex Bennée
2019-06-17 13:13 ` Alex Bennée
5 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2019-06-17 10:17 UTC (permalink / raw)
To: qemu-devel
vandersonmr <vandersonmr2@gmail.com> writes:
> This is the first series of patches related to the TCGCodeQuality GSoC project
> More at https://wiki.qemu.org/Features/TCGCodeQuality
>
> It adds an option to instrument TBs and collects their execution frequency.
> The execution frequency is then store/accumulated in an auxiliary structure
> every time a tb_flush or a read happens.
>
> [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
> [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
> [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.
Did you explicitly set the patch prefix to qemu-devel? You don't need
to, they are added by the mailing list software.
--
Alex Bennée
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user vandersonmr
2019-06-17 2:54 ` Richard Henderson
@ 2019-06-17 11:30 ` Alex Bennée
1 sibling, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2019-06-17 11:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, vandersonmr, Laurent Vivier, Markus Armbruster
[added Markus to Cc for his view on options]
vandersonmr <vandersonmr2@gmail.com> writes:
> Added -execfreq to enable execution frequency counting and dump
> all the TB's addresses and their execution frequency at the end
> of the execution.
>
> Signed-off-by: vandersonmr <vandersonmr2@gmail.com>
This works well enough but we are going to need a way to enable this for
softmmu as well. The preference for that is to add a option group to
qemu-options.hx which will allow a number of related options to be
grouped together.
The last thing that was added was the thread=multi flag to -accel
tcg,thread=multi but unfortunately the -accel option is very much a
softmmu only option group.
Maybe it's time to add a -tcg option for all the various options so we
can have (and are likely to add)?:
-tcg tbcount=true
-tcg tbcount=true,tbstats=file,file=output.txt
-tcg tbcount=true,tbstats=chardev,id=statschar
There are going to future enhancements we might consider:
-tcg jitperf=/tmp/perf.map
A long time in the future we may even have:
-tcg multiexitblocks=true
until it defaults to true and the knob is just there to turn it off.
The closest example of an option group shared between linux-user and
softmmu is the trace code. Both builds do a:
qemu_add_opts(&qemu_trace_opts);
(why can't this be done by module init code?)
And then they can call the general qemu_opts parser on the top level
option:
trace_opt_parse(arg);
So maybe we need to bite the bullet and create tcg/tcg-options.c and put
our tweaking machinery in there (and the real location of
enable_freq_count)?
Markus do you have a view?
> ---
> linux-user/exit.c | 5 +++++
> linux-user/main.c | 7 +++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/linux-user/exit.c b/linux-user/exit.c
> index bdda720553..0c6a2f2d5b 100644
> --- a/linux-user/exit.c
> +++ b/linux-user/exit.c
> @@ -26,8 +26,13 @@
> extern void __gcov_dump(void);
> #endif
>
> +extern bool enable_freq_count;
> +
> void preexit_cleanup(CPUArchState *env, int code)
> {
> + if (enable_freq_count) {
> + tb_dump_all_exec_freq();
> + }
> #ifdef TARGET_GPROF
> _mcleanup();
> #endif
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 1bf7155670..ece2d8bd8b 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -388,6 +388,11 @@ static void handle_arg_trace(const char *arg)
> trace_file = trace_opt_parse(arg);
> }
>
> +static void handle_arg_execfreq(const char *arg)
> +{
> + enable_freq_count = true;
> +}
> +
> struct qemu_argument {
> const char *argv;
> const char *env;
> @@ -439,6 +444,8 @@ static const struct qemu_argument arg_table[] = {
> "", "Seed for pseudo-random number generator"},
> {"trace", "QEMU_TRACE", true, handle_arg_trace,
> "", "[[enable=]<pattern>][,events=<file>][,file=<file>]"},
> + {"execfreq", "QEMU_EXEC_FREQ", false, handle_arg_execfreq,
> + "", "enable and dump TB's execution frequency counting"},
> {"version", "QEMU_VERSION", false, handle_arg_version,
> "", "display version information and exit"},
> {NULL, NULL, false, NULL, NULL, NULL}
--
Alex Bennée
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
` (4 preceding siblings ...)
2019-06-17 10:17 ` Alex Bennée
@ 2019-06-17 13:13 ` Alex Bennée
5 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2019-06-17 13:13 UTC (permalink / raw)
To: qemu-devel
vandersonmr <vandersonmr2@gmail.com> writes:
> This is the first series of patches related to the TCGCodeQuality GSoC project
> More at https://wiki.qemu.org/Features/TCGCodeQuality
>
> It adds an option to instrument TBs and collects their execution frequency.
> The execution frequency is then store/accumulated in an auxiliary structure
> every time a tb_flush or a read happens.
>
> [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.
> [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events.
> [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user.
One more thing:
https://app.shippable.com/github/stsquad/qemu/runs/866/summary/console
The use of:
uint64_t exec_freq;
breaks 32 bit builds as we violate ATOMIC_REG_SIZE. Maybe we can get
away with uint32_t? I guess we need more of an idea of the range of
these counters are likely to hit (and maybe detect overflow in the
helper?).
--
Alex Bennée
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-17 13:17 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 13:53 [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency vandersonmr
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter vandersonmr
2019-06-17 2:28 ` Richard Henderson
2019-06-17 10:01 ` Alex Bennée
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events vandersonmr
2019-06-17 2:52 ` Richard Henderson
2019-06-17 10:00 ` Alex Bennée
2019-06-14 13:53 ` [Qemu-devel] [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user vandersonmr
2019-06-17 2:54 ` Richard Henderson
2019-06-17 11:30 ` Alex Bennée
2019-06-14 16:01 ` [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency no-reply
2019-06-17 10:17 ` Alex Bennée
2019-06-17 13:13 ` Alex Bennée
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).