Re: [PATCH 1/8] accel/tcg: cache the result of curr_cflags()

On 8/18/26 10:42, Matt Turner wrote:
> 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.
> 
> Compute the value once into CPUState::tcg_curr_cflags and recompute it
> from the four places that can change an input: tcg_cflags_set(),
> cpu_single_step(), tcg_set_one_insn_per_tb() and qemu_set_log_internal().
> curr_cflags() becomes a single load.

Certainly one_insn_per_tb can only be set from the command-line, so that should never have 
been handled along this path.

cpu_single_stepping is one load and test.  I agree it can be done along the path that sets 
single stepping though.

LOG_TB_NOCHAIN can be set from command-line or monitor.  The first obviously can be set up 
before we get started.  The second can be handled via stop-the-world.

> + * Catch a cached value that has gone stale because an input changed without
> + * a matching tcg_update_curr_cflags().  Called from curr_cflags() on the
> + * dispatch path, so it exists only in debug-tcg builds.
> + */
> +void tcg_assert_curr_cflags(CPUState *cpu)
> +{
> +    uint32_t cached = cpu->tcg_curr_cflags;
> +    uint32_t fresh = compute_curr_cflags(cpu);
> +
> +    if (unlikely(cached != fresh)) {
> +        fprintf(stderr, "stale tcg_curr_cflags on CPU %d: "
> +                "cached 0x%08x, recomputed 0x%08x (differ in 0x%08x)\n",
> +                cpu->cpu_index, cached, fresh, cached ^ fresh);
> +        g_assert_not_reached();
> +    }
> +}
> +#endif

But this seems like overkill.


r~