diff mbox series

[v3,06/20] plugins: Use emit_before_op for PLUGIN_GEN_AFTER_INSN

Message ID 20240424230224.941028-7-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series Rewrite plugin code generation | expand

Commit Message

Richard Henderson April 24, 2024, 11:02 p.m. UTC 
Introduce a new plugin_cb op and migrate one operation.
By using emit_before_op, we do not need to emit opcodes
early and modify them later -- we can simply emit the
final set of opcodes once.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/tcg/tcg-op-common.h |  1 +
 include/tcg/tcg-opc.h       |  1 +
 accel/tcg/plugin-gen.c      | 74 +++++++++++++++++++++----------------
 tcg/tcg-op.c                |  5 +++
 4 files changed, 50 insertions(+), 31 deletions(-)

Comments

Pierrick Bouvier April 29, 2024, 7:56 p.m. UTC | #1 
On 4/24/24 16:02, Richard Henderson wrote:
> Introduce a new plugin_cb op and migrate one operation.
> By using emit_before_op, we do not need to emit opcodes
> early and modify them later -- we can simply emit the
> final set of opcodes once.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/tcg/tcg-op-common.h |  1 +
>   include/tcg/tcg-opc.h       |  1 +
>   accel/tcg/plugin-gen.c      | 74 +++++++++++++++++++++----------------
>   tcg/tcg-op.c                |  5 +++
>   4 files changed, 50 insertions(+), 31 deletions(-)
> 
> diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
> index 2d932a515e..9de5a7f280 100644
> --- a/include/tcg/tcg-op-common.h
> +++ b/include/tcg/tcg-op-common.h
> @@ -74,6 +74,7 @@ void tcg_gen_goto_tb(unsigned idx);
>    */
>   void tcg_gen_lookup_and_goto_ptr(void);
>   
> +void tcg_gen_plugin_cb(unsigned from);
>   void tcg_gen_plugin_cb_start(unsigned from, unsigned type, unsigned wr);
>   void tcg_gen_plugin_cb_end(void);
>   
> diff --git a/include/tcg/tcg-opc.h b/include/tcg/tcg-opc.h
> index b80227fa1c..3b7cb2bce1 100644
> --- a/include/tcg/tcg-opc.h
> +++ b/include/tcg/tcg-opc.h
> @@ -197,6 +197,7 @@ DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
>   DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
>   DEF(goto_ptr, 0, 1, 0, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
>   
> +DEF(plugin_cb, 0, 0, 1, TCG_OPF_NOT_PRESENT)
>   DEF(plugin_cb_start, 0, 0, 3, TCG_OPF_NOT_PRESENT)
>   DEF(plugin_cb_end, 0, 0, 0, TCG_OPF_NOT_PRESENT)
>   
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 4b488943ff..4b02c0bfbf 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -201,8 +201,7 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
>   {
>       switch (from) {
>       case PLUGIN_GEN_AFTER_INSN:
> -        gen_wrapped(from, PLUGIN_GEN_DISABLE_MEM_HELPER,
> -                    gen_empty_mem_helper);
> +        tcg_gen_plugin_cb(from);
>           break;
>       case PLUGIN_GEN_FROM_INSN:
>           /*
> @@ -608,16 +607,6 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
>       inject_mem_helper(begin_op, arr);
>   }
>   
> -static void inject_mem_disable_helper(struct qemu_plugin_insn *plugin_insn,
> -                                      TCGOp *begin_op)
> -{
> -    if (likely(!plugin_insn->mem_helper)) {
> -        rm_ops(begin_op);
> -        return;
> -    }
> -    inject_mem_helper(begin_op, NULL);
> -}
> -
>   /* called before finishing a TB with exit_tb, goto_tb or goto_ptr */
>   void plugin_gen_disable_mem_helpers(void)
>   {
> @@ -703,11 +692,14 @@ static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
>       inject_mem_enable_helper(ptb, insn, begin_op);
>   }
>   
> -static void plugin_gen_disable_mem_helper(struct qemu_plugin_tb *ptb,
> -                                          TCGOp *begin_op, int insn_idx)
> +static void gen_disable_mem_helper(struct qemu_plugin_tb *ptb,
> +                                   struct qemu_plugin_insn *insn)
>   {
> -    struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> -    inject_mem_disable_helper(insn, begin_op);
> +    if (insn->mem_helper) {
> +        tcg_gen_st_ptr(tcg_constant_ptr(0), tcg_env,
> +                       offsetof(CPUState, plugin_mem_cbs) -
> +                       offsetof(ArchCPU, env));
> +    }
>   }
>   
>   /* #define DEBUG_PLUGIN_GEN_OPS */
> @@ -766,16 +758,49 @@ static void pr_ops(void)
>   
>   static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>   {
> -    TCGOp *op;
> +    TCGOp *op, *next;
>       int insn_idx = -1;
>   
>       pr_ops();
>   
> -    QTAILQ_FOREACH(op, &tcg_ctx->ops, link) {
> +    /*
> +     * While injecting code, we cannot afford to reuse any ebb temps
> +     * that might be live within the existing opcode stream.
> +     * The simplest solution is to release them all and create new.
> +     */
> +    memset(tcg_ctx->free_temps, 0, sizeof(tcg_ctx->free_temps));
> +
> +    QTAILQ_FOREACH_SAFE(op, &tcg_ctx->ops, link, next) {
>           switch (op->opc) {
>           case INDEX_op_insn_start:
>               insn_idx++;
>               break;
> +
> +        case INDEX_op_plugin_cb:
> +        {
> +            enum plugin_gen_from from = op->args[0];
> +            struct qemu_plugin_insn *insn = NULL;
> +
> +            if (insn_idx >= 0) {
> +                insn = g_ptr_array_index(plugin_tb->insns, insn_idx);
> +            }
> +
> +            tcg_ctx->emit_before_op = op;
> +
> +            switch (from) {
> +            case PLUGIN_GEN_AFTER_INSN:
> +                assert(insn != NULL);
> +                gen_disable_mem_helper(plugin_tb, insn);
> +                break;
> +            default:
> +                g_assert_not_reached();
> +            }
> +
> +            tcg_ctx->emit_before_op = NULL;
> +            tcg_op_remove(tcg_ctx, op);
> +            break;
> +        }
> +
>           case INDEX_op_plugin_cb_start:
>           {
>               enum plugin_gen_from from = op->args[0];
> @@ -840,19 +865,6 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>   
>                   break;
>               }
> -            case PLUGIN_GEN_AFTER_INSN:
> -            {
> -                g_assert(insn_idx >= 0);
> -
> -                switch (type) {
> -                case PLUGIN_GEN_DISABLE_MEM_HELPER:
> -                    plugin_gen_disable_mem_helper(plugin_tb, op, insn_idx);
> -                    break;
> -                default:
> -                    g_assert_not_reached();
> -                }
> -                break;
> -            }
>               default:
>                   g_assert_not_reached();
>               }
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index aa6bc6f57d..0f2026c91c 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -312,6 +312,11 @@ void tcg_gen_mb(TCGBar mb_type)
>       }
>   }
>   
> +void tcg_gen_plugin_cb(unsigned from)
> +{
> +    tcg_gen_op1(INDEX_op_plugin_cb, from);
> +}
> +
>   void tcg_gen_plugin_cb_start(unsigned from, unsigned type, unsigned wr)
>   {
>       tcg_gen_op3(INDEX_op_plugin_cb_start, from, type, wr);

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

Patch

diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
index 2d932a515e..9de5a7f280 100644
--- a/include/tcg/tcg-op-common.h
+++ b/include/tcg/tcg-op-common.h
@@ -74,6 +74,7 @@  void tcg_gen_goto_tb(unsigned idx);
  */
 void tcg_gen_lookup_and_goto_ptr(void);
 
+void tcg_gen_plugin_cb(unsigned from);
 void tcg_gen_plugin_cb_start(unsigned from, unsigned type, unsigned wr);
 void tcg_gen_plugin_cb_end(void);
 
diff --git a/include/tcg/tcg-opc.h b/include/tcg/tcg-opc.h
index b80227fa1c..3b7cb2bce1 100644
--- a/include/tcg/tcg-opc.h
+++ b/include/tcg/tcg-opc.h
@@ -197,6 +197,7 @@  DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
 DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
 DEF(goto_ptr, 0, 1, 0, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
 
+DEF(plugin_cb, 0, 0, 1, TCG_OPF_NOT_PRESENT)
 DEF(plugin_cb_start, 0, 0, 3, TCG_OPF_NOT_PRESENT)
 DEF(plugin_cb_end, 0, 0, 0, TCG_OPF_NOT_PRESENT)
 
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 4b488943ff..4b02c0bfbf 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -201,8 +201,7 @@  static void plugin_gen_empty_callback(enum plugin_gen_from from)
 {
     switch (from) {
     case PLUGIN_GEN_AFTER_INSN:
-        gen_wrapped(from, PLUGIN_GEN_DISABLE_MEM_HELPER,
-                    gen_empty_mem_helper);
+        tcg_gen_plugin_cb(from);
         break;
     case PLUGIN_GEN_FROM_INSN:
         /*
@@ -608,16 +607,6 @@  static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
     inject_mem_helper(begin_op, arr);
 }
 
-static void inject_mem_disable_helper(struct qemu_plugin_insn *plugin_insn,
-                                      TCGOp *begin_op)
-{
-    if (likely(!plugin_insn->mem_helper)) {
-        rm_ops(begin_op);
-        return;
-    }
-    inject_mem_helper(begin_op, NULL);
-}
-
 /* called before finishing a TB with exit_tb, goto_tb or goto_ptr */
 void plugin_gen_disable_mem_helpers(void)
 {
@@ -703,11 +692,14 @@  static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
     inject_mem_enable_helper(ptb, insn, begin_op);
 }
 
-static void plugin_gen_disable_mem_helper(struct qemu_plugin_tb *ptb,
-                                          TCGOp *begin_op, int insn_idx)
+static void gen_disable_mem_helper(struct qemu_plugin_tb *ptb,
+                                   struct qemu_plugin_insn *insn)
 {
-    struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-    inject_mem_disable_helper(insn, begin_op);
+    if (insn->mem_helper) {
+        tcg_gen_st_ptr(tcg_constant_ptr(0), tcg_env,
+                       offsetof(CPUState, plugin_mem_cbs) -
+                       offsetof(ArchCPU, env));
+    }
 }
 
 /* #define DEBUG_PLUGIN_GEN_OPS */
@@ -766,16 +758,49 @@  static void pr_ops(void)
 
 static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
 {
-    TCGOp *op;
+    TCGOp *op, *next;
     int insn_idx = -1;
 
     pr_ops();
 
-    QTAILQ_FOREACH(op, &tcg_ctx->ops, link) {
+    /*
+     * While injecting code, we cannot afford to reuse any ebb temps
+     * that might be live within the existing opcode stream.
+     * The simplest solution is to release them all and create new.
+     */
+    memset(tcg_ctx->free_temps, 0, sizeof(tcg_ctx->free_temps));
+
+    QTAILQ_FOREACH_SAFE(op, &tcg_ctx->ops, link, next) {
         switch (op->opc) {
         case INDEX_op_insn_start:
             insn_idx++;
             break;
+
+        case INDEX_op_plugin_cb:
+        {
+            enum plugin_gen_from from = op->args[0];
+            struct qemu_plugin_insn *insn = NULL;
+
+            if (insn_idx >= 0) {
+                insn = g_ptr_array_index(plugin_tb->insns, insn_idx);
+            }
+
+            tcg_ctx->emit_before_op = op;
+
+            switch (from) {
+            case PLUGIN_GEN_AFTER_INSN:
+                assert(insn != NULL);
+                gen_disable_mem_helper(plugin_tb, insn);
+                break;
+            default:
+                g_assert_not_reached();
+            }
+
+            tcg_ctx->emit_before_op = NULL;
+            tcg_op_remove(tcg_ctx, op);
+            break;
+        }
+
         case INDEX_op_plugin_cb_start:
         {
             enum plugin_gen_from from = op->args[0];
@@ -840,19 +865,6 @@  static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
 
                 break;
             }
-            case PLUGIN_GEN_AFTER_INSN:
-            {
-                g_assert(insn_idx >= 0);
-
-                switch (type) {
-                case PLUGIN_GEN_DISABLE_MEM_HELPER:
-                    plugin_gen_disable_mem_helper(plugin_tb, op, insn_idx);
-                    break;
-                default:
-                    g_assert_not_reached();
-                }
-                break;
-            }
             default:
                 g_assert_not_reached();
             }
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index aa6bc6f57d..0f2026c91c 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -312,6 +312,11 @@  void tcg_gen_mb(TCGBar mb_type)
     }
 }
 
+void tcg_gen_plugin_cb(unsigned from)
+{
+    tcg_gen_op1(INDEX_op_plugin_cb, from);
+}
+
 void tcg_gen_plugin_cb_start(unsigned from, unsigned type, unsigned wr)
 {
     tcg_gen_op3(INDEX_op_plugin_cb_start, from, type, wr);