Most targets define their restore_state_to_opc() handler in cpu.c.
In order to keep SPARC aligned, move sparc_restore_state_to_opc()
from translate.c to cpu.c.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/cpu.h       | 11 ++++++++---
 target/sparc/cpu.c       | 23 +++++++++++++++++++++++
 target/sparc/translate.c | 32 --------------------------------
 3 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index f517e5a383..bcb3566a92 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -607,12 +607,17 @@ int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
                               uint8_t *buf, int len, bool is_write);
 #endif
 
+/* Dynamic PC, must exit to main loop. */
+#define DYNAMIC_PC         1
+/* Dynamic PC, one of two values according to jump_pc[T2]. */
+#define JUMP_PC            2
+/* Dynamic PC, may lookup next TB. */
+#define DYNAMIC_PC_LOOKUP  3
+
+#define DISAS_EXIT  DISAS_TARGET_0
 
 /* translate.c */
 void sparc_tcg_init(void);
-void sparc_restore_state_to_opc(CPUState *cs,
-                                const TranslationBlock *tb,
-                                const uint64_t *data);
 
 /* fop_helper.c */
 target_ulong cpu_get_fsr(CPUSPARCState *);
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index dd7af86de7..59db8c8472 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -713,6 +713,29 @@ static void sparc_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.npc = tb->cs_base;
 }
 
+static void sparc_restore_state_to_opc(CPUState *cs,
+                                       const TranslationBlock *tb,
+                                       const uint64_t *data)
+{
+    CPUSPARCState *env = cpu_env(cs);
+    target_ulong pc = data[0];
+    target_ulong npc = data[1];
+
+    env->pc = pc;
+    if (npc == DYNAMIC_PC) {
+        /* dynamic NPC: already stored */
+    } else if (npc & JUMP_PC) {
+        /* jump PC: use 'cond' and the jump targets of the translation */
+        if (env->cond) {
+            env->npc = npc & ~3;
+        } else {
+            env->npc = pc + 4;
+        }
+    } else {
+        env->npc = npc;
+    }
+}
+
 static bool sparc_cpu_has_work(CPUState *cs)
 {
     return (cs->interrupt_request & CPU_INTERRUPT_HARD) &&
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index cdd0a95c03..9942e78412 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -101,15 +101,6 @@
 # define MAXTL_MASK                             0
 #endif
 
-/* Dynamic PC, must exit to main loop. */
-#define DYNAMIC_PC         1
-/* Dynamic PC, one of two values according to jump_pc[T2]. */
-#define JUMP_PC            2
-/* Dynamic PC, may lookup next TB. */
-#define DYNAMIC_PC_LOOKUP  3
-
-#define DISAS_EXIT  DISAS_TARGET_0
-
 /* global register indexes */
 static TCGv_ptr cpu_regwptr;
 static TCGv cpu_pc, cpu_npc;
@@ -5881,26 +5872,3 @@ void sparc_tcg_init(void)
                                          gregnames[i]);
     }
 }
-
-void sparc_restore_state_to_opc(CPUState *cs,
-                                const TranslationBlock *tb,
-                                const uint64_t *data)
-{
-    CPUSPARCState *env = cpu_env(cs);
-    target_ulong pc = data[0];
-    target_ulong npc = data[1];
-
-    env->pc = pc;
-    if (npc == DYNAMIC_PC) {
-        /* dynamic NPC: already stored */
-    } else if (npc & JUMP_PC) {
-        /* jump PC: use 'cond' and the jump targets of the translation */
-        if (env->cond) {
-            env->npc = npc & ~3;
-        } else {
-            env->npc = pc + 4;
-        }
-    } else {
-        env->npc = npc;
-    }
-}
-- 
2.45.2

On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> Most targets define their restore_state_to_opc() handler in cpu.c.
> In order to keep SPARC aligned, move sparc_restore_state_to_opc()
> from translate.c to cpu.c.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/sparc/cpu.h       | 11 ++++++++---
>   target/sparc/cpu.c       | 23 +++++++++++++++++++++++
>   target/sparc/translate.c | 32 --------------------------------
>   3 files changed, 31 insertions(+), 35 deletions(-)
> 
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index f517e5a383..bcb3566a92 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -607,12 +607,17 @@ int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>                                 uint8_t *buf, int len, bool is_write);
>   #endif
>   
> +/* Dynamic PC, must exit to main loop. */
> +#define DYNAMIC_PC         1
> +/* Dynamic PC, one of two values according to jump_pc[T2]. */
> +#define JUMP_PC            2
> +/* Dynamic PC, may lookup next TB. */
> +#define DYNAMIC_PC_LOOKUP  3

Keep these out of cpu.h.
But frankly, moving the sparc_restore_state_to_opc to an "internal.h" kind of header is 
just as effective and keeps all of the private-to-translate.c things contained.

> +
> +#define DISAS_EXIT  DISAS_TARGET_0

Definitely shouldn't be moved.


r~

On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Most targets define their restore_state_to_opc() handler in cpu.c.
> In order to keep SPARC aligned, move sparc_restore_state_to_opc()
> from translate.c to cpu.c.
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/sparc/cpu.h       | 11 ++++++++---
>  target/sparc/cpu.c       | 23 +++++++++++++++++++++++
>  target/sparc/translate.c | 32 --------------------------------
>  3 files changed, 31 insertions(+), 35 deletions(-)
>
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index f517e5a383..bcb3566a92 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -607,12 +607,17 @@ int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>                                uint8_t *buf, int len, bool is_write);
>  #endif
>
> +/* Dynamic PC, must exit to main loop. */
> +#define DYNAMIC_PC         1
> +/* Dynamic PC, one of two values according to jump_pc[T2]. */
> +#define JUMP_PC            2
> +/* Dynamic PC, may lookup next TB. */
> +#define DYNAMIC_PC_LOOKUP  3
> +
> +#define DISAS_EXIT  DISAS_TARGET_0

Why move the definition of DISAS_EXIT ?
sparc_restore_state_to_opc() doesn't use it, and
the DISAS_ constants are a translate-time-only concept.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM