Matt Turner <[email protected]> writes:
> 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.
I agree this would be useful to remove but...
>
> static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
> diff --git ./include/hw/core/cpu.h ./include/hw/core/cpu.h
> index b54035fb13..172872d005 100644
> --- ./include/hw/core/cpu.h
> +++ ./include/hw/core/cpu.h
> @@ -411,10 +411,16 @@ struct qemu_work_item;
> * to a cluster this will be UNASSIGNED_CLUSTER_INDEX; otherwise it will
> * be the same as the cluster-id property of the CPU object's
> TYPE_CPU_CLUSTER
> * QOM parent.
> - * Under TCG this value is propagated to @tcg_cflags.
> + * Under TCG this value is propagated to @tcg_cflags_priv.
> * See TranslationBlock::TCG CF_CLUSTER_MASK.
> * @start_powered_off: Indicates whether the CPU starts in powered-off state.
> - * @tcg_cflags: Pre-computed cflags for this cpu.
> + * @tcg_cflags_priv: Pre-computed cflags for this cpu. Private to
> + * tcg_cflags_has() and tcg_cflags_set(): @tcg_curr_cflags is derived from
> + * it and is refreshed by the setter, so a direct assignment here would
> + * leave the two out of step. The name is deliberately awkward to make an
> + * open-coded access fail to compile rather than silently go stale.
> + * @tcg_curr_cflags: Cached result of curr_cflags(), recomputed by
> + * tcg_update_curr_cflags() whenever any of its inputs change.
This seems like introducing additional cache state to track. Why not
just provide a helper to call when ss/gdb/log conditions change and
change the current curr_cflags() code to only validate the helper has
been called when TCG debugging is turned on. See ppc_get_tb_cpu_state or
assert_hflags_rebuild_correctly for examples.
> * @nr_threads: Number of threads within this CPU core.
> * @thread: Host thread details, only live once @created is #true
> * @sem: WIN32 only semaphore used only for qtest
> @@ -557,7 +563,8 @@ struct CPUState {
> /* TODO Move common fields from CPUArchState here. */
> int cpu_index;
> int cluster_index;
> - uint32_t tcg_cflags;
> + uint32_t tcg_cflags_priv;
> + uint32_t tcg_curr_cflags;
> uint32_t halted;
> int32_t exception_index;
>
<snip>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
