On 10/7/2026 11:11 PM, Akihiko Odaki wrote: > On 2026/10/08 10:08, Pierrick Bouvier wrote: >> On 10/6/2026 11:01 PM, Akihiko Odaki wrote: >>> A region of interest (ROI) often excludes the program startup and exit. >>> For example, a benchmark may parse a large input and then run a >>> computation kernel and the former may take longer than the latter; >>> basic block vectors generated for the whole execution will not properly >>> represent the latter. >>> >>> Add options to specify the beginning and the end of the ROI to bound >>> basic block vector generation to the ROI. >>> >>> Signed-off-by: Akihiko Odaki <[email protected]> >>> --- >>> docs/about/emulation.rst | 8 +++++ >>> contrib/plugins/bbv.c | 80 +++++++++++++++++++++++++++++++++++++ >>> +++++++++-- >>> 2 files changed, 86 insertions(+), 2 deletions(-) >>> >>> diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst >>> index c58149086cdd..0566171b517b 100644 >>> --- a/docs/about/emulation.rst >>> +++ b/docs/about/emulation.rst >>> @@ -284,6 +284,14 @@ The bbv plugin allows you to generate basic >>> block vectors for use with the >>> * - Option >>> - Description >>> + * - begin=A >>> + - The virtual address of the instruction that starts basic block >>> vector >>> + generation. Generation starts from the beginning of the >>> execution if >>> + not specified. >>> + * - end=A >>> + - The virtual address of the instruction that ends basic block >>> vector >>> + generation. Generation continues until the endo of the >>> execution if >> >> small typo here: s/endo/end > > Thanks for catching the typo. > >> >>> + not specified. >>> * - interval=N >>> - The interval to generate a basic block vector specified by >>> the number of >>> instructions (Default: N = 100000000) >>> diff --git a/contrib/plugins/bbv.c b/contrib/plugins/bbv.c >>> index e5caf5d9543b..a2a779f7639e 100644 >>> --- a/contrib/plugins/bbv.c >>> +++ b/contrib/plugins/bbv.c >>> @@ -5,6 +5,8 @@ >>> * SPDX-License-Identifier: GPL-2.0-or-later >>> */ >>> +#include <stdbool.h> >>> +#include <stdint.h> >>> #include <stdio.h> >>> #include <glib.h> >>> @@ -26,6 +28,10 @@ static GHashTable *bbs; >>> static GRWLock bbs_lock; >>> static char *filename; >>> static struct qemu_plugin_scoreboard *vcpus; >>> +static bool begin_present; >>> +static uint64_t begin; >>> +static bool end_present; >>> +static uint64_t end; >>> static int64_t interval = 100000000; >>> static void plugin_exit(void *p) >>> @@ -66,6 +72,7 @@ static void vcpu_init(unsigned int vcpu_index, void >>> *userdata) >>> Vcpu *vcpu = qemu_plugin_scoreboard_find(vcpus, vcpu_index); >>> vcpu_filename = g_strdup_printf("%s.%u.bb", filename, >>> vcpu_index); >>> + vcpu->count = begin_present ? INT64_MIN : -interval; >> >> Not sure which semantic we are trying to achieve with -interval here. >> >> The original logic was to increment this count, until reaching interval, >> after what we would reset it by using vcpu_count -= interval, to keep >> the extra insn that went in after the threshold. > > The counter now starts at -interval and counts up to zero. Subtracting > interval after emitting a vector still preserves the extra instructions. > For example, with interval=100: > > initial count: -100 > after 130 insns: 30 -> emit a vector > subtract interval: -70 -> 70 insns until the next vector > >> >>> vcpu->file = fopen(vcpu_filename, "w"); >>> } >>> @@ -102,6 +109,22 @@ static void vcpu_interval_exec(unsigned int >>> vcpu_index, void *udata) >>> fputc('\n', vcpu->file); >>> } >>> +static void vcpu_begin_insn_exec(unsigned int vcpu_index, void >>> *userdata) >>> +{ >>> + GHashTableIter iter; >>> + void *value; >>> + >>> + g_rw_lock_reader_lock(&bbs_lock); >>> + g_hash_table_iter_init(&iter, bbs); >>> + >>> + while (g_hash_table_iter_next(&iter, NULL, &value)) { >>> + qemu_plugin_u64_set(bb_count_u64(value), vcpu_index, 0); >>> + } >>> + >>> + g_rw_lock_reader_unlock(&bbs_lock); >>> + qemu_plugin_u64_set(count_u64(), vcpu_index, -interval); >>> +} >>> + >>> static void vcpu_tb_trans(struct qemu_plugin_tb *tb, void *userdata) >>> { >>> uint64_t n_insns = qemu_plugin_tb_n_insns(tb); >>> @@ -119,6 +142,19 @@ static void vcpu_tb_trans(struct qemu_plugin_tb >>> *tb, void *userdata) >>> } >>> g_rw_lock_writer_unlock(&bbs_lock); >>> + for (size_t idx = 0; idx < n_insns; idx++) { >>> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, >>> idx); >>> + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn); >>> + >>> + if (begin_present && insn_vaddr == begin) { >>> + qemu_plugin_register_vcpu_insn_exec_cb( >>> + insn, vcpu_begin_insn_exec, QEMU_PLUGIN_CB_NO_REGS, >>> NULL); >>> + } else if (end_present && insn_vaddr == end) { >>> + qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu( >>> + insn, QEMU_PLUGIN_INLINE_STORE_U64, count_u64(), >>> INT64_MIN); >>> + } >>> + } >>> + >>> qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu( >>> tb, QEMU_PLUGIN_INLINE_ADD_U64, count_u64(), n_insns); >>> @@ -127,7 +163,32 @@ static void vcpu_tb_trans(struct >>> qemu_plugin_tb *tb, void *userdata) >>> qemu_plugin_register_vcpu_tb_exec_cond_cb( >>> tb, vcpu_interval_exec, QEMU_PLUGIN_CB_NO_REGS, >>> - QEMU_PLUGIN_COND_GE, count_u64(), interval, NULL); >>> + QEMU_PLUGIN_COND_LT, count_u64(), INT64_MIN, NULL); >>> +} >> >> is INT64_MIN used as an arbitrary large value when interpreted as >> unsigned immediate? If yes, that's confusing to the reader. > > The unsigned comparison count < (uint64_t)INT64_MIN is equivalent with > the signed comparison (int64_t)count >= 0. That is why this particular > value is used. > > Setting count to INT64_MIN suppresses output for the next 2^63 counted > instructions, which is effectively disabled for this use. The begin > callback clears the BB counters and sets count to -interval; end sets it > back to INT64_MIN. The requested interval stays unchanged. >
Ok I see better. Even though it works, it's pretty confusing to be honest. Inline operatons works with unsigned values, so with conversion and wrap around, it's safe, but really not intuitive. I would prefer if you could refactor this to keep a zero initialized count value, and add a specific instrumentation path for begin, end logic (COND_LT, UINT64_MAX - N) to keep the same behavior of "infinite interval". Thanks, Pierrick
