On 17/9/26 10:26, Richard Henderson wrote:
From: Matt Turner <[email protected]>
curr_cflags() is called once per TB dispatch, from helper_lookup_tb_ptr()
and from the cpu_exec() loop. It recomputes the same value every time:
uint32_t cflags = cpu->tcg_cflags;
if (unlikely(cpu_single_stepping(cpu))) { ... }
else if (qatomic_read(&one_insn_per_tb)) { ... }
else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { ... }
That is three loads and three branches on the hottest path in the
interpreter, for state that changes only when gdb enables single-step,
when one-insn-per-tb is toggled, or when the log mask changes.
None of the three has to be sampled at dispatch time. Fold each into
CPUState::tcg_cflags where it changes and curr_cflags() becomes a single
load of a field that TB lookup has to read anyway.
The derived bits -- CF_COUNT_MASK, CF_NO_GOTO_TB, CF_NO_GOTO_PTR and
CF_SINGLE_STEP -- are never set by tcg_cflags_set(), so tcg_update_cflags()
can recompute them in place without disturbing the rest, and conversely
tcg_cflags_set() ORs in its bits without disturbing them.
There are three places to call it:
- tcg_exec_realizefn(), so that a CPU created after the command line has
been parsed starts out with the right value. This covers user-only,
where tcg_cpu_init_cflags() is not reached. linux-user's cpu_copy()
copies tcg_cflags wholesale, so a cloned thread inherits it.
- cpu_single_step(), which changes one CPU. gdb is the only caller that
matters; in system mode it runs with the vCPUs stopped, and in user mode
gdb_continue_partial() can reach a thread that is still running, because
gdb_handlesig() stops only the thread that trapped. That is exactly the
plain cross-thread store to another CPU's CPUState that
cpu->singlestep_flags already was, read back by that CPU through
cpu_single_stepping() in curr_cflags(). This patch changes which field
carries it, not who writes it or how.
- hmp_one_insn_per_tb() and hmp_log(), which change every CPU while the
vCPUs are running, so the update is queued with async_run_on_cpu() and
each CPU writes its own cflags from its own thread. The command line
spellings of those two settings need nothing: they are parsed before
any CPU is realized, so tcg_exec_realizefn() picks them up.
Noticed this commit while rebasing hybrid-accel work. Could this new
helper be useful for this previous discussion (patches 38-40)?
https://lore.kernel.org/qemu-devel/[email protected]/
Measured with qemu-alpha running an emulated alpha gcc 16.2.0 compiling
the SQLite 3.45.1 amalgamation (255k lines, -O2) on an x86-64 host, in a
build configured with --enable-lto:
before: 1,646,994,254,249 instructions
after: 1,562,204,796,597 instructions -5.15%
That workload issues 8.4 billion dispatches, so the per-call saving is
small but the aggregate is not. The emulated compiler produces
byte-identical output before and after.
Wall clock does not move: 133.19s to 132.58s, a 0.46% difference against a
run-to-run spread larger than that. The removed work is a few predictable
loads and branches that the host executes largely in parallel with the
surrounding dispatch, so this patch is worth taking for the instruction
count and for what it enables, not for a time saving that can be measured
on its own.
v4: Update the cflags from the HMP handlers for 'log' and 'one-insn-per-tb'
rather than from qemu_set_log_internal() and the accelerator property
setter. Those are the paths that reach a running vCPU, and the monitor
is the only thing that does. Suggested by Richard Henderson.
v4: Queue the per-CPU update with async_run_on_cpu() rather than
async_safe_run_on_cpu(). Halting the other vCPUs buys nothing: the
queued work already runs on the owning CPU's own thread. Suggested by
Alex Bennee, who also asked whether there are cross-vCPU updates of
tcg_cflags at all. With this change the monitor path has none: the
only remaining writer from another thread is cpu_single_step(), above,
which is neither new nor made worse here.
v4: Move the stub to accel/stubs/, which is where the other accelerator
stubs live.
Signed-off-by: Matt Turner <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
Signed-off-by: Richard Henderson <[email protected]>
Message-ID: <[email protected]>
---
accel/tcg/internal-common.h | 11 +++++++++--
include/system/tcg.h | 12 ++++++++++++
accel/tcg/cpu-exec-common.c | 33 ++++++++++++++++++++++++++++++---
accel/tcg/cpu-exec.c | 3 +++
accel/tcg/tcg-accel-ops.c | 2 ++
cpu-target.c | 5 ++++-
monitor/hmp-cmds.c | 7 +++++++
system/runstate-hmp-cmds.c | 18 ++++++++++++------
8 files changed, 79 insertions(+), 12 deletions(-)
