diff mbox

[RFC,v2,02/34] cpu-exec: Purge all uses of CPU_GET_ENV

Message ID ec6b5d43c6c33c22a0706541930b1d6eabeb173a.1433052532.git.crosthwaite.peter@gmail.com
State New
Headers show

Commit Message

Peter Crosthwaite May 31, 2015, 6:11 a.m. UTC 
Remove un-needed usages of CPU_GET_ENV by converting the APIs to use
CPUState pointers and retrieving the env_ptr as minimally needed.

FIXME: apply target-foo change pattern to all archs.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 cpu-exec.c              | 28 +++++++++++++---------------
 cpus.c                  |  3 +--
 target-arm/cpu.h        |  2 +-
 target-microblaze/cpu.h |  2 +-
 4 files changed, 16 insertions(+), 19 deletions(-)

Comments

Richard Henderson June 1, 2015, 7:03 p.m. UTC | #1 
On 05/30/2015 11:11 PM, Peter Crosthwaite wrote:
> Remove un-needed usages of CPU_GET_ENV by converting the APIs to use
> CPUState pointers and retrieving the env_ptr as minimally needed.
> 
> FIXME: apply target-foo change pattern to all archs.

Yes, please.

>  #ifdef TARGET_I386
>      X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>  #endif

  CPUArchState *env = &x86_cpu.env;

>  #ifdef TARGET_I386
> +            env = cpu->env_ptr;
>              x86_cpu = X86_CPU(cpu);
>  #endif

Likewise.

Or, indeed, it might be worth propagating away "env" entirely, so you don't
have to reload it here at the end of the setjmp.

Otherwise this starter patch looks good.


r~
Peter Crosthwaite June 7, 2015, 10:57 p.m. UTC | #2 
On Mon, Jun 1, 2015 at 12:03 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 05/30/2015 11:11 PM, Peter Crosthwaite wrote:
>> Remove un-needed usages of CPU_GET_ENV by converting the APIs to use
>> CPUState pointers and retrieving the env_ptr as minimally needed.
>>
>> FIXME: apply target-foo change pattern to all archs.
>
> Yes, please.
>

Fixed.

>>  #ifdef TARGET_I386
>>      X86CPU *x86_cpu = X86_CPU(cpu);
>> +    CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>>  #endif
>
>   CPUArchState *env = &x86_cpu.env;
>

Fixed.

>>  #ifdef TARGET_I386
>> +            env = cpu->env_ptr;
>>              x86_cpu = X86_CPU(cpu);
>>  #endif
>
> Likewise.
>

And fixed.

> Or, indeed, it might be worth propagating away "env" entirely, so you don't
> have to reload it here at the end of the setjmp.
>

I'm leaving this as follow-up for the moment as it is a bit of a
change to target-i386 code. cpu_svm_check_intercept_param() needs to
be de-envified but it has a fair few internal uses in target-x86. I'm
thinking it should take a CPU * (not an X86CPU) due to its call from
common code. This means we should be able to stubbify the function and
remove the TARGET_I386 ifeffery entirely one day. The drawback is
adding QOM casts in target-i386. Is this thing ever a fast path? Will
other arches want to use this hook one day as well?

Regards,
Peter

> Otherwise this starter patch looks good.
>
>
> r~
>
>
diff mbox

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index 2ffeb6e..0266609 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -226,10 +226,9 @@  static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, uint8_t *tb_ptr)
 
 /* Execute the code without caching the generated code. An interpreter
    could be used if available. */
-static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
+static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
                              TranslationBlock *orig_tb)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
     TranslationBlock *tb;
     target_ulong pc = orig_tb->pc;
     target_ulong cs_base = orig_tb->cs_base;
@@ -253,12 +252,12 @@  static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
     tb_free(tb);
 }
 
-static TranslationBlock *tb_find_slow(CPUArchState *env,
+static TranslationBlock *tb_find_slow(CPUState *cpu,
                                       target_ulong pc,
                                       target_ulong cs_base,
                                       uint64_t flags)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
+    CPUArchState *env = (CPUArchState *)cpu->env_ptr;
     TranslationBlock *tb, **ptb1;
     unsigned int h;
     tb_page_addr_t phys_pc, phys_page1;
@@ -310,9 +309,9 @@  static TranslationBlock *tb_find_slow(CPUArchState *env,
     return tb;
 }
 
-static inline TranslationBlock *tb_find_fast(CPUArchState *env)
+static inline TranslationBlock *tb_find_fast(CPUState *cpu)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
+    CPUArchState *env = (CPUArchState *)cpu->env_ptr;
     TranslationBlock *tb;
     target_ulong cs_base, pc;
     int flags;
@@ -324,14 +323,13 @@  static inline TranslationBlock *tb_find_fast(CPUArchState *env)
     tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
     if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
                  tb->flags != flags)) {
-        tb = tb_find_slow(env, pc, cs_base, flags);
+        tb = tb_find_slow(cpu, pc, cs_base, flags);
     }
     return tb;
 }
 
-static void cpu_handle_debug_exception(CPUArchState *env)
+static void cpu_handle_debug_exception(CPUState *cpu)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
     CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUWatchpoint *wp;
 
@@ -348,12 +346,12 @@  static void cpu_handle_debug_exception(CPUArchState *env)
 
 volatile sig_atomic_t exit_request;
 
-int cpu_exec(CPUArchState *env)
+int cpu_exec(CPUState *cpu)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
     CPUClass *cc = CPU_GET_CLASS(cpu);
 #ifdef TARGET_I386
     X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUArchState *env = (CPUArchState *)cpu->env_ptr;
 #endif
     int ret, interrupt_request;
     TranslationBlock *tb;
@@ -406,7 +404,7 @@  int cpu_exec(CPUArchState *env)
                     /* exit request from the cpu execution loop */
                     ret = cpu->exception_index;
                     if (ret == EXCP_DEBUG) {
-                        cpu_handle_debug_exception(env);
+                        cpu_handle_debug_exception(cpu);
                     }
                     cpu->exception_index = -1;
                     break;
@@ -482,7 +480,7 @@  int cpu_exec(CPUArchState *env)
                 }
                 spin_lock(&tcg_ctx.tb_ctx.tb_lock);
                 have_tb_lock = true;
-                tb = tb_find_fast(env);
+                tb = tb_find_fast(cpu);
                 /* Note: we do it here to avoid a gcc bug on Mac OS X when
                    doing it in tb_find_slow */
                 if (tcg_ctx.tb_ctx.tb_invalidated_flag) {
@@ -542,7 +540,7 @@  int cpu_exec(CPUArchState *env)
                             if (insns_left > 0) {
                                 /* Execute remaining instructions.  */
                                 tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
-                                cpu_exec_nocache(env, insns_left, tb);
+                                cpu_exec_nocache(cpu, insns_left, tb);
                                 align_clocks(&sc, cpu);
                             }
                             cpu->exception_index = EXCP_INTERRUPT;
@@ -566,10 +564,10 @@  int cpu_exec(CPUArchState *env)
             /* Reload env after longjmp - the compiler may have smashed all
              * local variables as longjmp is marked 'noreturn'. */
             cpu = current_cpu;
-            env = cpu->env_ptr;
             cc = CPU_GET_CLASS(cpu);
             cpu->can_do_io = 1;
 #ifdef TARGET_I386
+            env = cpu->env_ptr;
             x86_cpu = X86_CPU(cpu);
 #endif
             if (have_tb_lock) {
diff --git a/cpus.c b/cpus.c
index 1b8f05a..c8a2911 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1316,7 +1316,6 @@  int vm_stop_force_state(RunState state)
 
 static int tcg_cpu_exec(CPUState *cpu)
 {
-    CPUArchState *env = cpu->env_ptr;
     int ret;
 #ifdef CONFIG_PROFILER
     int64_t ti;
@@ -1351,7 +1350,7 @@  static int tcg_cpu_exec(CPUState *cpu)
         cpu->icount_decr.u16.low = decr;
         cpu->icount_extra = count;
     }
-    ret = cpu_exec(env);
+    ret = cpu_exec(cpu);
 #ifdef CONFIG_PROFILER
     tcg_time += profile_getclock() - ti;
 #endif
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d4a5899..5a4cd84 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -488,7 +488,7 @@  typedef struct CPUARMState {
 #include "cpu-qom.h"
 
 ARMCPU *cpu_arm_init(const char *cpu_model);
-int cpu_arm_exec(CPUARMState *s);
+int cpu_arm_exec(CPUState *cpu);
 uint32_t do_arm_semihosting(CPUARMState *env);
 void aarch64_sync_32_to_64(CPUARMState *env);
 void aarch64_sync_64_to_32(CPUARMState *env);
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 4ea04ac..d2dfeb4 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -276,7 +276,7 @@  struct CPUMBState {
 
 void mb_tcg_init(void);
 MicroBlazeCPU *cpu_mb_init(const char *cpu_model);
-int cpu_mb_exec(CPUMBState *s);
+int cpu_mb_exec(CPUState *cpu);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
    signal handlers to inform the virtual CPU of exceptions. non zero
    is returned if the signal was handled by the virtual CPU.  */