diff mbox series

[PATCH-for-10.1,10/12] tcg: Unify tcg_gen_insn_start() to handle 1 or 2 arguments

Message ID 20250319134507.45045-11-philmd@linaro.org
State New
Headers show
Series tcg: Make tcg_gen_insn_start() target-agnostic | expand

Commit Message

Philippe Mathieu-Daudé March 19, 2025, 1:45 p.m. UTC 
Merge the tcg_gen_insn_start() definition using 1 extra word
with the definition using 2, using a2=0 in callers.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/tcg/tcg-op.h          | 16 ++++------------
 target/i386/tcg/translate.c   |  2 +-
 target/m68k/translate.c       |  2 +-
 target/microblaze/translate.c |  2 +-
 target/openrisc/translate.c   |  2 +-
 target/sh4/translate.c        |  2 +-
 target/sparc/translate.c      |  2 +-
 7 files changed, 10 insertions(+), 18 deletions(-)

Comments

Richard Henderson March 19, 2025, 4:37 p.m. UTC | #1 
On 3/19/25 06:45, Philippe Mathieu-Daudé wrote:
> Merge the tcg_gen_insn_start() definition using 1 extra word
> with the definition using 2, using a2=0 in callers.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/tcg/tcg-op.h          | 16 ++++------------
>   target/i386/tcg/translate.c   |  2 +-
>   target/m68k/translate.c       |  2 +-
>   target/microblaze/translate.c |  2 +-
>   target/openrisc/translate.c   |  2 +-
>   target/sh4/translate.c        |  2 +-
>   target/sparc/translate.c      |  2 +-
>   7 files changed, 10 insertions(+), 18 deletions(-)

Nack.

> @@ -50,7 +40,9 @@ static inline void tcg_gen_insn_start(uint64_t pc, uint64_t a1, uint64_t a2)
>   
>       tcg_set_insn_start_param(op, 0, pc);
>       tcg_set_insn_start_param(op, 1, a1);
> -    tcg_set_insn_start_param(op, 2, a2);
> +    if (insn_start_words > 2) {
> +        tcg_set_insn_start_param(op, 2, a2);
> +    }

Not this, where you're passing unused arguments but for some reason are not storing them.

If you want to unify all callers on 3 arguments, that's one thing.
But conditionally not storing them?  That's just weird.

If you *do* want to unify all callers on 3 arguments, then you can dispense with 
TARGET_INSN_START_EXTRA_WORDS entirely.  Just define TCG_INSN_START_WORDS == 3 generically 
and drop everything else.


r~
diff mbox series

Patch

diff --git a/include/tcg/tcg-op.h b/include/tcg/tcg-op.h
index 0915043adf8..df234d4e1e0 100644
--- a/include/tcg/tcg-op.h
+++ b/include/tcg/tcg-op.h
@@ -31,17 +31,7 @@  static inline void tcg_gen_insn_start(uint64_t pc)
 
     tcg_set_insn_start_param(op, 0, pc);
 }
-#elif TARGET_INSN_START_EXTRA_WORDS == 1
-static inline void tcg_gen_insn_start(uint64_t pc, uint64_t a1)
-{
-    unsigned insn_start_words = tcg_ctx->insn_start_words;
-    TCGOp *op = tcg_emit_op(INDEX_op_insn_start,
-                            insn_start_words * 64 / TCG_TARGET_REG_BITS);
-
-    tcg_set_insn_start_param(op, 0, pc);
-    tcg_set_insn_start_param(op, 1, a1);
-}
-#elif TARGET_INSN_START_EXTRA_WORDS >= 2
+#elif TARGET_INSN_START_EXTRA_WORDS >= 1
 static inline void tcg_gen_insn_start(uint64_t pc, uint64_t a1, uint64_t a2)
 {
     unsigned insn_start_words = tcg_ctx->insn_start_words;
@@ -50,7 +40,9 @@  static inline void tcg_gen_insn_start(uint64_t pc, uint64_t a1, uint64_t a2)
 
     tcg_set_insn_start_param(op, 0, pc);
     tcg_set_insn_start_param(op, 1, a1);
-    tcg_set_insn_start_param(op, 2, a2);
+    if (insn_start_words > 2) {
+        tcg_set_insn_start_param(op, 2, a2);
+    }
 }
 #endif
 
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index a8935f487aa..0f1e72b1bc0 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -3766,7 +3766,7 @@  static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
     if (tb_cflags(dcbase->tb) & CF_PCREL) {
         pc_arg &= ~TARGET_PAGE_MASK;
     }
-    tcg_gen_insn_start(pc_arg, dc->cc_op);
+    tcg_gen_insn_start(pc_arg, dc->cc_op, 0);
 }
 
 static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index dec2967fce4..9f7052aa6b5 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6037,7 +6037,7 @@  static void m68k_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
 static void m68k_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
-    tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);
+    tcg_gen_insn_start(dc->base.pc_next, dc->cc_op, 0);
 }
 
 static void m68k_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index b54e5ac4b2f..d256b317cfb 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1631,7 +1631,7 @@  static void mb_tr_insn_start(DisasContextBase *dcb, CPUState *cs)
 {
     DisasContext *dc = container_of(dcb, DisasContext, base);
 
-    tcg_gen_insn_start(dc->base.pc_next, dc->tb_flags & ~MSR_TB_MASK);
+    tcg_gen_insn_start(dc->base.pc_next, dc->tb_flags & ~MSR_TB_MASK, 0);
 }
 
 static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 7a6af183ae2..ff720e7fb6b 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1558,7 +1558,7 @@  static void openrisc_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
     tcg_gen_insn_start(dc->base.pc_next, (dc->delayed_branch ? 1 : 0)
-                       | (dc->base.num_insns > 1 ? 2 : 0));
+                       | (dc->base.num_insns > 1 ? 2 : 0), 0);
 }
 
 static void openrisc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index bcdd5588183..f140080c08c 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2248,7 +2248,7 @@  static void sh4_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
-    tcg_gen_insn_start(ctx->base.pc_next, ctx->envflags);
+    tcg_gen_insn_start(ctx->base.pc_next, ctx->envflags, 0);
 }
 
 static void sh4_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index bfe63649db2..a775f425658 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5725,7 +5725,7 @@  static void sparc_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
             g_assert_not_reached();
         }
     }
-    tcg_gen_insn_start(dc->pc, npc);
+    tcg_gen_insn_start(dc->pc, npc, 0);
 }
 
 static void sparc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)