diff mbox

[v2,22/45] cpu-exec: lookup/generate TB outside exclusive region during step_atomic

Message ID 1500235468-15341-23-git-send-email-cota@braap.org (mailing list archive)
State New, archived
Headers show

Commit Message

Emilio Cota July 16, 2017, 8:04 p.m. UTC
Now that all code generation has been converted to check CF_PARALLEL, we can
generate !CF_PARALLEL code without having yet set !parallel_cpus --
and therefore without having to be in the exclusive region during
cpu_exec_step_atomic.

While at it, merge cpu_exec_step into cpu_exec_step_atomic.

Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 accel/tcg/cpu-exec.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

Comments

Richard Henderson July 18, 2017, 12:01 a.m. UTC | #1
On 07/16/2017 10:04 AM, Emilio G. Cota wrote:
> Now that all code generation has been converted to check CF_PARALLEL, we can
> generate !CF_PARALLEL code without having yet set !parallel_cpus --
> and therefore without having to be in the exclusive region during
> cpu_exec_step_atomic.
> 
> While at it, merge cpu_exec_step into cpu_exec_step_atomic.
> 
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>   accel/tcg/cpu-exec.c | 26 ++++++++++++--------------
>   1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index efe5c85..23e6d2c 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -226,7 +226,7 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
>   }
>   #endif
>   
> -static void cpu_exec_step(CPUState *cpu)
> +void cpu_exec_step_atomic(CPUState *cpu)
>   {
>       CPUClass *cc = CPU_GET_CLASS(cpu);
>       TranslationBlock *tb;
> @@ -239,16 +239,26 @@ static void cpu_exec_step(CPUState *cpu)
>           if (tb == NULL) {
>               mmap_lock();
>               tb_lock();
> -            tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);
> +            tb = tb_htable_lookup(cpu, pc, cs_base, flags, mask_cf(cflags));
> +            if (likely(tb == NULL)) {
> +                tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);
> +            }
>               tb_unlock();
>               mmap_unlock();
>           }
>   
> +        start_exclusive();
> +
> +        /* Since we got here, we know that parallel_cpus must be true.  */
> +        parallel_cpus = false;

Well, since we've moved parallel_cpus completely out of target/*, we no longer 
have to set this false, right?

I wonder how hard it would be to completely hide this variable now...
That said, even that would probably be better as a follow-on cleanup.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
diff mbox

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index efe5c85..23e6d2c 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -226,7 +226,7 @@  static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
 }
 #endif
 
-static void cpu_exec_step(CPUState *cpu)
+void cpu_exec_step_atomic(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
     TranslationBlock *tb;
@@ -239,16 +239,26 @@  static void cpu_exec_step(CPUState *cpu)
         if (tb == NULL) {
             mmap_lock();
             tb_lock();
-            tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);
+            tb = tb_htable_lookup(cpu, pc, cs_base, flags, mask_cf(cflags));
+            if (likely(tb == NULL)) {
+                tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);
+            }
             tb_unlock();
             mmap_unlock();
         }
 
+        start_exclusive();
+
+        /* Since we got here, we know that parallel_cpus must be true.  */
+        parallel_cpus = false;
         cc->cpu_exec_enter(cpu);
         /* execute the generated code */
         trace_exec_tb(tb, pc);
         cpu_tb_exec(cpu, tb);
         cc->cpu_exec_exit(cpu);
+        parallel_cpus = true;
+
+        end_exclusive();
     } else {
         /* We may have exited due to another problem here, so we need
          * to reset any tb_locks we may have taken but didn't release.
@@ -262,18 +272,6 @@  static void cpu_exec_step(CPUState *cpu)
     }
 }
 
-void cpu_exec_step_atomic(CPUState *cpu)
-{
-    start_exclusive();
-
-    /* Since we got here, we know that parallel_cpus must be true.  */
-    parallel_cpus = false;
-    cpu_exec_step(cpu);
-    parallel_cpus = true;
-
-    end_exclusive();
-}
-
 struct tb_desc {
     target_ulong pc;
     target_ulong cs_base;