Re: [PATCH 17/22] plugins: Replace pr_ops with a proper debug dump flag

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/16/24 05:57, Richard Henderson wrote:
> The DEBUG_PLUGIN_GEN_OPS ifdef is replaced with "-d op_plugin".
> The second pr_ops call can be obtained with "-d op".
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/qemu/log.h     |  1 +
>   include/tcg/tcg.h      |  1 +
>   accel/tcg/plugin-gen.c | 68 ++++++++----------------------------------
>   tcg/tcg.c              | 29 +++++++++++++++++-
>   util/log.c             |  4 +++
>   5 files changed, 46 insertions(+), 57 deletions(-)
> 
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index df59bfabcd..e10e24cd4f 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -36,6 +36,7 @@ bool qemu_log_separate(void);
>   #define LOG_STRACE         (1 << 19)
>   #define LOG_PER_THREAD     (1 << 20)
>   #define CPU_LOG_TB_VPU     (1 << 21)
> +#define LOG_TB_OP_PLUGIN   (1 << 22)
>   
>   /* Lock/unlock output. */
>   
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index df66e8f012..753d7ca3e0 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -1065,5 +1065,6 @@ static inline const TCGOpcode *tcg_swap_vecop_list(const TCGOpcode *n)
>   }
>   
>   bool tcg_can_emit_vecop_list(const TCGOpcode *, TCGType, unsigned);
> +void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs);
>   
>   #endif /* TCG_H */
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 6f0731b479..10d917abd3 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -44,6 +44,7 @@
>    */
>   #include "qemu/osdep.h"
>   #include "qemu/plugin.h"
> +#include "qemu/log.h"
>   #include "cpu.h"
>   #include "tcg/tcg.h"
>   #include "tcg/tcg-temp-internal.h"
> @@ -58,6 +59,7 @@
>   # define CONFIG_SOFTMMU_GATE 0
>   #endif
>   
> +/* Update plugin_from_name in tcg.c. */
>   enum plugin_gen_from {
>       PLUGIN_GEN_FROM_TB,
>       PLUGIN_GEN_FROM_INSN,
> @@ -192,66 +194,21 @@ static void gen_mem_cb(struct qemu_plugin_dyn_cb *cb,
>       tcg_temp_free_i32(cpu_index);
>   }
>   
> -/* #define DEBUG_PLUGIN_GEN_OPS */
> -static void pr_ops(void)
> -{
> -#ifdef DEBUG_PLUGIN_GEN_OPS
> -    TCGOp *op;
> -    int i = 0;
> -
> -    QTAILQ_FOREACH(op, &tcg_ctx->ops, link) {
> -        const char *name = "";
> -        const char *type = "";
> -
> -        if (op->opc == INDEX_op_plugin_cb_start) {
> -            switch (op->args[0]) {
> -            case PLUGIN_GEN_FROM_TB:
> -                name = "tb";
> -                break;
> -            case PLUGIN_GEN_FROM_INSN:
> -                name = "insn";
> -                break;
> -            case PLUGIN_GEN_FROM_MEM:
> -                name = "mem";
> -                break;
> -            case PLUGIN_GEN_AFTER_INSN:
> -                name = "after insn";
> -                break;
> -            default:
> -                break;
> -            }
> -            switch (op->args[1]) {
> -            case PLUGIN_GEN_CB_UDATA:
> -                type = "udata";
> -                break;
> -            case PLUGIN_GEN_CB_INLINE:
> -                type = "inline";
> -                break;
> -            case PLUGIN_GEN_CB_MEM:
> -                type = "mem";
> -                break;
> -            case PLUGIN_GEN_ENABLE_MEM_HELPER:
> -                type = "enable mem helper";
> -                break;
> -            case PLUGIN_GEN_DISABLE_MEM_HELPER:
> -                type = "disable mem helper";
> -                break;
> -            default:
> -                break;
> -            }
> -        }
> -        printf("op[%2i]: %s %s %s\n", i, tcg_op_defs[op->opc].name, name, type);
> -        i++;
> -    }
> -#endif
> -}
> -
>   static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>   {
>       TCGOp *op, *next;
>       int insn_idx = -1;
>   
> -    pr_ops();
> +    if (unlikely(qemu_loglevel_mask(LOG_TB_OP_PLUGIN)
> +                 && qemu_log_in_addr_range(plugin_tb->vaddr))) {
> +        FILE *logfile = qemu_log_trylock();
> +        if (logfile) {
> +            fprintf(logfile, "OP before plugin injection:\n");
> +            tcg_dump_ops(tcg_ctx, logfile, false);
> +            fprintf(logfile, "\n");
> +            qemu_log_unlock(logfile);
> +        }
> +    }
>   
>       /*
>        * While injecting code, we cannot afford to reuse any ebb temps
> @@ -389,7 +346,6 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>               break;
>           }
>       }
> -    pr_ops();
>   }
>   
>   bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 363a065e28..d248c52e96 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -2540,6 +2540,15 @@ static const char bswap_flag_name[][6] = {
>       [TCG_BSWAP_IZ | TCG_BSWAP_OS] = "iz,os",
>   };
>   
> +#ifdef CONFIG_PLUGIN
> +static const char * const plugin_from_name[] = {
> +    "from-tb",
> +    "from-insn",
> +    "after-insn",
> +    "after-tb",
> +};
> +#endif
> +
>   static inline bool tcg_regset_single(TCGRegSet d)
>   {
>       return (d & (d - 1)) == 0;
> @@ -2558,7 +2567,7 @@ static inline TCGReg tcg_regset_first(TCGRegSet d)
>   #define ne_fprintf(...) \
>       ({ int ret_ = fprintf(__VA_ARGS__); ret_ >= 0 ? ret_ : 0; })
>   
> -static void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
> +void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
>   {
>       char buf[128];
>       TCGOp *op;
> @@ -2714,6 +2723,24 @@ static void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
>                       i = k = 1;
>                   }
>                   break;
> +#ifdef CONFIG_PLUGIN
> +            case INDEX_op_plugin_cb:
> +                {
> +                    TCGArg from = op->args[k++];
> +                    const char *name = NULL;
> +
> +                    if (from < ARRAY_SIZE(plugin_from_name)) {
> +                        name = plugin_from_name[from];
> +                    }
> +                    if (name) {
> +                        col += ne_fprintf(f, "%s", name);
> +                    } else {
> +                        col += ne_fprintf(f, "$0x%" TCG_PRIlx, from);
> +                    }
> +                    i = 1;
> +                }
> +                break;
> +#endif
>               default:
>                   i = 0;
>                   break;
> diff --git a/util/log.c b/util/log.c
> index d36c98da0b..6219819855 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -466,6 +466,10 @@ const QEMULogItem qemu_log_items[] = {
>         "show micro ops after optimization" },
>       { CPU_LOG_TB_OP_IND, "op_ind",
>         "show micro ops before indirect lowering" },
> +#ifdef CONFIG_PLUGIN
> +    { LOG_TB_OP_PLUGIN, "op_plugin",
> +      "show micro ops before plugin injection" },
> +#endif
>       { CPU_LOG_INT, "int",
>         "show interrupts/exceptions in short format" },
>       { CPU_LOG_EXEC, "exec",

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>