Re: [PATCH 20/22] plugins: Move qemu_plugin_insn_cleanup_fn to tcg.c

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> This is only used in one place, and usage requires an
> out-of-line function.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/qemu/plugin.h | 12 ------------
>  tcg/tcg.c             | 12 ++++++++++++
>  2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index 07b1755990..201889cbee 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -116,18 +116,6 @@ struct qemu_plugin_scoreboard {
>      QLIST_ENTRY(qemu_plugin_scoreboard) entry;
>  };
>  
> -/*
> - * qemu_plugin_insn allocate and cleanup functions. We don't expect to
> - * cleanup many of these structures. They are reused for each fresh
> - * translation.
> - */
> -
> -static inline void qemu_plugin_insn_cleanup_fn(gpointer data)
> -{
> -    struct qemu_plugin_insn *insn = (struct qemu_plugin_insn *) data;
> -    g_byte_array_free(insn->data, true);
> -}
> -
>  /* Internal context for this TranslationBlock */
>  struct qemu_plugin_tb {
>      GPtrArray *insns;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index d248c52e96..d7abc514c4 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -761,6 +761,18 @@ QEMU_BUILD_BUG_ON((int)(offsetof(CPUNegativeOffsetState, tlb.f[0]) -
>                    < MIN_TLB_MASK_TABLE_OFS);
>  #endif
>  
> +#ifdef CONFIG_PLUGIN
> +/*
> + * We don't expect to cleanup many of these structures.
> + * They are reused for each fresh translation.
> + */
> +static void qemu_plugin_insn_cleanup_fn(gpointer data)
> +{
> +    struct qemu_plugin_insn *insn = (struct qemu_plugin_insn *) data;
> +    g_byte_array_free(insn->data, true);
> +}
> +#endif
> +

You could expand the ifdef to the next function and make an alternate
empty alloc_tcg_plugin_context. Alternatively maybe we should consider
dropping the CONFIG_PLUGIN gymnastics and make it a first class TCG
feature?

Is the null case still visible on the code generation benchmarks? Does
anyone using TCG actually --disable-plugins?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>  static void alloc_tcg_plugin_context(TCGContext *s)
>  {
>  #ifdef CONFIG_PLUGIN

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro