[Qemu-devel] [PATCH 0/10] Factorize code in translate.c

[Qemu-devel] [PATCH 0/10] Factorize code in translate.c

[Glauber Costa]

Hi,

This series is an (initial) attempt to make qemu code a little bit
more modular. In this patch series specifically, I'm removing
some #ifdef TARGET_* from the general-purpose translate.c,
and rather, replacing them with a common function implemented in
architecture specific files.

Any rants and complaints, let me know

Hope you like it

[Qemu-devel] [PATCH 01/10] i386: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 exec-all.h              |    3 +++
 target-i386/translate.c |   23 +++++++++++++++++++++++
 translate-all.c         |   22 +---------------------
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 62b8191..3ce8242 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -61,6 +61,9 @@ extern int loglevel;
 
 int gen_intermediate_code(CPUState *env, struct TranslationBlock *tb);
 int gen_intermediate_code_pc(CPUState *env, struct TranslationBlock *tb);
+void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
+                 unsigned long searched_pc, int pc_pos, void *puc);
+
 unsigned long code_gen_max_block_size(void);
 void cpu_gen_init(void);
 int cpu_gen_code(CPUState *env, struct TranslationBlock *tb,
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 356ceff..895d14b 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -6832,3 +6832,26 @@ int gen_intermediate_code_pc(CPUState *env, TranslationBlock *tb)
     return gen_intermediate_code_internal(env, tb, 1);
 }
 
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    int cc_op;
+#ifdef DEBUG_DISAS
+    if (loglevel & CPU_LOG_TB_OP) {
+        int i;
+        fprintf(logfile, "RESTORE:\n");
+        for(i = 0;i <= pc_pos; i++) {
+            if (gen_opc_instr_start[i]) {
+                fprintf(logfile, "0x%04x: " TARGET_FMT_lx "\n", i, gen_opc_pc[i]);
+            }
+        }
+        fprintf(logfile, "spc=0x%08lx pc_pos=0x%x eip=" TARGET_FMT_lx " cs_base=%x\n",
+                searched_pc, pc_pos, gen_opc_pc[pc_pos] - tb->cs_base,
+                (uint32_t)tb->cs_base);
+    }
+#endif
+    env->eip = gen_opc_pc[pc_pos] - tb->cs_base;
+    cc_op = gen_opc_cc_op[pc_pos];
+    if (cc_op != CC_OP_DYNAMIC)
+        env->cc_op = cc_op;
+}
diff --git a/translate-all.c b/translate-all.c
index 6a273a8..73e1d67 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -194,27 +194,7 @@ int cpu_restore_state(TranslationBlock *tb,
     while (gen_opc_instr_start[j] == 0)
         j--;
 #if defined(TARGET_I386)
-    {
-        int cc_op;
-#ifdef DEBUG_DISAS
-        if (loglevel & CPU_LOG_TB_OP) {
-            int i;
-            fprintf(logfile, "RESTORE:\n");
-            for(i=0;i<=j; i++) {
-                if (gen_opc_instr_start[i]) {
-                    fprintf(logfile, "0x%04x: " TARGET_FMT_lx "\n", i, gen_opc_pc[i]);
-                }
-            }
-            fprintf(logfile, "spc=0x%08lx j=0x%x eip=" TARGET_FMT_lx " cs_base=%x\n",
-                    searched_pc, j, gen_opc_pc[j] - tb->cs_base,
-                    (uint32_t)tb->cs_base);
-        }
-#endif
-        env->eip = gen_opc_pc[j] - tb->cs_base;
-        cc_op = gen_opc_cc_op[j];
-        if (cc_op != CC_OP_DYNAMIC)
-            env->cc_op = cc_op;
-    }
+    gen_pc_load(env, tb, searched_pc, j, puc);
 #elif defined(TARGET_ARM)
     env->regs[15] = gen_opc_pc[j];
 #elif defined(TARGET_SPARC)
-- 
1.5.0.6

[Qemu-devel] [PATCH 02/10] arm: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-arm/translate.c |    5 +++++
 translate-all.c        |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6de78f8..8966996 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7706,3 +7706,8 @@ void cpu_dump_state(CPUState *env, FILE *f,
     cpu_fprintf(f, "FPSCR: %08x\n", (int)env->vfp.xregs[ARM_VFP_FPSCR]);
 }
 
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    env->regs[15] = gen_opc_pc[pc_pos];
+}
diff --git a/translate-all.c b/translate-all.c
index 73e1d67..e688318 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -196,7 +196,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #if defined(TARGET_I386)
     gen_pc_load(env, tb, searched_pc, j, puc);
 #elif defined(TARGET_ARM)
-    env->regs[15] = gen_opc_pc[j];
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_SPARC)
     {
         target_ulong npc;
-- 
1.5.0.6

[Qemu-devel] [PATCH 03/10] sparc: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-sparc/translate.c |   20 ++++++++++++++++++++
 translate-all.c          |   18 +-----------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 90d9278..6edf64d 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -4659,3 +4659,23 @@ void gen_intermediate_code_init(CPUSPARCState *env)
                                               gregnames[i]);
     }
 }
+
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    target_ulong npc;
+    env->pc = gen_opc_pc[pc_pos];
+    npc = gen_opc_npc[pc_pos];
+    if (npc == 1) {
+        /* dynamic NPC: already stored */
+    } else if (npc == 2) {
+        target_ulong t2 = (target_ulong)(unsigned long)puc;
+        /* jump PC: use T2 and the jump targets of the translation */
+        if (t2)
+            env->npc = gen_opc_jump_pc[0];
+        else
+            env->npc = gen_opc_jump_pc[1];
+    } else {
+        env->npc = npc;
+    }
+}
diff --git a/translate-all.c b/translate-all.c
index e688318..92c8f1c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -198,23 +198,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_ARM)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_SPARC)
-    {
-        target_ulong npc;
-        env->pc = gen_opc_pc[j];
-        npc = gen_opc_npc[j];
-        if (npc == 1) {
-            /* dynamic NPC: already stored */
-        } else if (npc == 2) {
-            target_ulong t2 = (target_ulong)(unsigned long)puc;
-            /* jump PC: use T2 and the jump targets of the translation */
-            if (t2)
-                env->npc = gen_opc_jump_pc[0];
-            else
-                env->npc = gen_opc_jump_pc[1];
-        } else {
-            env->npc = npc;
-        }
-    }
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_PPC)
     {
         int type, c;
-- 
1.5.0.6

[Qemu-devel] [PATCH 04/10] ppc: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-ppc/translate.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 translate-all.c        |   40 +---------------------------------------
 2 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index c9530eb..6f7d16c 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6344,3 +6344,45 @@ int gen_intermediate_code_pc (CPUState *env, struct TranslationBlock *tb)
 {
     return gen_intermediate_code_internal(env, tb, 1);
 }
+
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    int type, c;
+    /* for PPC, we need to look at the micro operation to get the
+     * access type */
+    env->nip = gen_opc_pc[pc_pos];
+    c = gen_opc_buf[pc_pos];
+    switch(c) {
+#if defined(CONFIG_USER_ONLY)
+#define CASE3(op)\
+    case INDEX_op_ ## op ## _raw
+#else
+#define CASE3(op)\
+    case INDEX_op_ ## op ## _user:\
+    case INDEX_op_ ## op ## _kernel:\
+    case INDEX_op_ ## op ## _hypv
+#endif
+
+    CASE3(stfd):
+    CASE3(stfs):
+    CASE3(lfd):
+    CASE3(lfs):
+        type = ACCESS_FLOAT;
+        break;
+    CASE3(lwarx):
+        type = ACCESS_RES;
+        break;
+    CASE3(stwcx):
+        type = ACCESS_RES;
+        break;
+    CASE3(eciwx):
+    CASE3(ecowx):
+        type = ACCESS_EXT;
+        break;
+    default:
+        type = ACCESS_INT;
+        break;
+    }
+    env->access_type = type;
+}
diff --git a/translate-all.c b/translate-all.c
index 92c8f1c..66f03e1 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -200,45 +200,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_SPARC)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_PPC)
-    {
-        int type, c;
-        /* for PPC, we need to look at the micro operation to get the
-           access type */
-        env->nip = gen_opc_pc[j];
-        c = gen_opc_buf[j];
-        switch(c) {
-#if defined(CONFIG_USER_ONLY)
-#define CASE3(op)\
-        case INDEX_op_ ## op ## _raw
-#else
-#define CASE3(op)\
-        case INDEX_op_ ## op ## _user:\
-        case INDEX_op_ ## op ## _kernel:\
-        case INDEX_op_ ## op ## _hypv
-#endif
-
-        CASE3(stfd):
-        CASE3(stfs):
-        CASE3(lfd):
-        CASE3(lfs):
-            type = ACCESS_FLOAT;
-            break;
-        CASE3(lwarx):
-            type = ACCESS_RES;
-            break;
-        CASE3(stwcx):
-            type = ACCESS_RES;
-            break;
-        CASE3(eciwx):
-        CASE3(ecowx):
-            type = ACCESS_EXT;
-            break;
-        default:
-            type = ACCESS_INT;
-            break;
-        }
-        env->access_type = type;
-    }
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_M68K)
     env->pc = gen_opc_pc[j];
 #elif defined(TARGET_MIPS)
-- 
1.5.0.6

[Qemu-devel] [PATCH 05/10] m68k: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-m68k/translate.c |    5 +++++
 translate-all.c         |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 39816f4..36e27a1 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3280,3 +3280,8 @@ void cpu_dump_state(CPUState *env, FILE *f,
     cpu_fprintf (f, "FPRESULT = %12g\n", *(double *)&env->fp_result);
 }
 
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    env->pc = gen_opc_pc[pc_pos];
+}
diff --git a/translate-all.c b/translate-all.c
index 66f03e1..5e63978 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -202,7 +202,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_PPC)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_M68K)
-    env->pc = gen_opc_pc[j];
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_MIPS)
     env->PC[env->current_tc] = gen_opc_pc[j];
     env->hflags &= ~MIPS_HFLAG_BMASK;
-- 
1.5.0.6

[Qemu-devel] [PATCH 06/10] mips: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-mips/translate.c |    8 ++++++++
 translate-all.c         |    4 +---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 65f0ba9..46b917e 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -6942,3 +6942,11 @@ void cpu_reset (CPUMIPSState *env)
 #endif
     cpu_mips_register(env, env->cpu_model);
 }
+
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    env->PC[env->current_tc] = gen_opc_pc[pc_pos];
+    env->hflags &= ~MIPS_HFLAG_BMASK;
+    env->hflags |= gen_opc_hflags[pc_pos];
+}
diff --git a/translate-all.c b/translate-all.c
index 5e63978..d2408f6 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -204,9 +204,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_M68K)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_MIPS)
-    env->PC[env->current_tc] = gen_opc_pc[j];
-    env->hflags &= ~MIPS_HFLAG_BMASK;
-    env->hflags |= gen_opc_hflags[j];
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_ALPHA)
     env->pc = gen_opc_pc[j];
 #elif defined(TARGET_SH4)
-- 
1.5.0.6

[Qemu-devel] [PATCH 07/10] alpha: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-alpha/translate.c |    5 +++++
 translate-all.c          |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 1c8587d..c7bb207 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2108,3 +2108,8 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
     return env;
 }
 
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    env->pc = gen_opc_pc[pc_pos];
+}
diff --git a/translate-all.c b/translate-all.c
index d2408f6..bcad1f4 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -206,7 +206,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_MIPS)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_ALPHA)
-    env->pc = gen_opc_pc[j];
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_SH4)
     env->pc = gen_opc_pc[j];
     env->flags = gen_opc_hflags[j];
-- 
1.5.0.6

[Qemu-devel] [PATCH 08/10] sh4: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-sh4/translate.c |    7 +++++++
 translate-all.c        |    3 +--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index b7c5f8d..1468e7a 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -1302,3 +1302,10 @@ int gen_intermediate_code_pc(CPUState * env, struct TranslationBlock *tb)
 {
     return gen_intermediate_code_internal(env, tb, 1);
 }
+
+void gen_pc_load(CPUState *env, TranslationBlock *tb,
+                unsigned long searched_pc, int pc_pos, void *puc)
+{
+    env->pc = gen_opc_pc[pc_pos];
+    env->flags = gen_opc_hflags[pc_pos];
+}
diff --git a/translate-all.c b/translate-all.c
index bcad1f4..401969c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -208,8 +208,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_ALPHA)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_SH4)
-    env->pc = gen_opc_pc[j];
-    env->flags = gen_opc_hflags[j];
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_CRIS)
     env->pregs[PR_ERP] = gen_opc_pc[j];
 #endif
-- 
1.5.0.6

[Qemu-devel] [PATCH 09/10] cris: replace code inside ifdef with generic function

[Glauber Costa]

this patch replaced code inside specific ifdef in translate-all.c
with a generic architecture-implemented function. This leads to a
cleaner and more modular code in the generic part.
---
 target-cris/translate.c |    6 ++++++
 translate-all.c         |    2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/target-cris/translate.c b/target-cris/translate.c
index 5769175..e522678 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -2701,3 +2701,9 @@ void cpu_reset (CPUCRISState *env)
 	memset(env, 0, offsetof(CPUCRISState, breakpoints));
 	tlb_flush(env, 1);
 }
+
+void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
+                 unsigned long searched_pc, int pc_pos, void *puc)
+{
+    env->pregs[PR_ERP] = gen_opc_pc[pc_pos];
+}
diff --git a/translate-all.c b/translate-all.c
index 401969c..3afb656 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -210,7 +210,7 @@ int cpu_restore_state(TranslationBlock *tb,
 #elif defined(TARGET_SH4)
     gen_pc_load(env, tb, searched_pc, j, puc); 
 #elif defined(TARGET_CRIS)
-    env->pregs[PR_ERP] = gen_opc_pc[j];
+    gen_pc_load(env, tb, searched_pc, j, puc); 
 #endif
 
 #ifdef CONFIG_PROFILER
-- 
1.5.0.6

[Qemu-devel] [PATCH 10/10] remove arch-specific ifdefs from translate-all.c

[Glauber Costa]

As the current state of matters, all architectures call a function
with the same name and signature for updating the pc. Remove the ifdefs,
leading to a cleaner code
---
 translate-all.c |   19 +------------------
 1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 3afb656..83056d5 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -193,25 +193,8 @@ int cpu_restore_state(TranslationBlock *tb,
     /* now find start of instruction before */
     while (gen_opc_instr_start[j] == 0)
         j--;
-#if defined(TARGET_I386)
+
     gen_pc_load(env, tb, searched_pc, j, puc);
-#elif defined(TARGET_ARM)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_SPARC)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_PPC)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_M68K)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_MIPS)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_ALPHA)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_SH4)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#elif defined(TARGET_CRIS)
-    gen_pc_load(env, tb, searched_pc, j, puc); 
-#endif
 
 #ifdef CONFIG_PROFILER
     dyngen_restore_time += profile_getclock() - ti;
-- 
1.5.0.6