Patchwork [08/15] Avoid declaring the env variable at all if CONFIG_TCG_PASS_AREG0.

login
register
mail settings
Submitter Richard Henderson
Date March 25, 2012, 10:27 p.m.
Message ID <1332714477-30079-9-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/148601/
State New
Headers show

Comments

Richard Henderson - March 25, 2012, 10:27 p.m.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 dyngen-exec.h |    5 +++++
 user-exec.c   |   17 ++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)
Blue Swirl - March 26, 2012, 4:26 p.m.
On Sun, Mar 25, 2012 at 22:27, Richard Henderson <rth@twiddle.net> wrote:
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  dyngen-exec.h |    5 +++++
>  user-exec.c   |   17 ++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/dyngen-exec.h b/dyngen-exec.h
> index cfeef99..65fcb43 100644
> --- a/dyngen-exec.h
> +++ b/dyngen-exec.h
> @@ -19,6 +19,10 @@
>  #if !defined(__DYNGEN_EXEC_H__)
>  #define __DYNGEN_EXEC_H__
>
> +/* If the target has indicated that it does not need an AREG0,
> +   don't declare the env variable at all, much less as a register.  */
> +#if !defined(CONFIG_TCG_PASS_AREG0)
> +
>  #if defined(CONFIG_TCG_INTERPRETER)
>  /* The TCG interpreter does not need a special register AREG0,
>  * but it is possible to use one by defining AREG0.
> @@ -65,4 +69,5 @@ register CPUArchState *env asm(AREG0);
>  extern CPUArchState *env;
>  #endif
>
> +#endif /* !CONFIG_TCG_PASS_AREG0 */
>  #endif /* !defined(__DYNGEN_EXEC_H__) */
> diff --git a/user-exec.c b/user-exec.c
> index cd905ff..e326104 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -58,7 +58,9 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
>     struct sigcontext *uc = puc;
>  #endif
>
> +#ifndef CONFIG_TCG_PASS_AREG0
>     env = env1;
> +#endif

Shouldn't longjmp() restore global registers as well? Actually, we
return to cpu-exec.c which does not use global env. Isn't this
useless?

>
>     /* XXX: restore cpu registers saved in host registers */
>
> @@ -74,8 +76,8 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
>         sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
>  #endif
>     }
> -    env->exception_index = -1;
> -    longjmp(env->jmp_env, 1);
> +    env1->exception_index = -1;
> +    longjmp(env1->jmp_env, 1);
>  }
>
>  /* 'pc' is the host PC at which the exception was raised. 'address' is
> @@ -89,9 +91,18 @@ static inline int handle_cpu_signal(unsigned long pc, unsigned long address,
>     TranslationBlock *tb;
>     int ret;
>
> +    /* XXX: find a correct solution for multithread */
> +#ifdef CONFIG_TCG_PASS_AREG0
> +    /* ??? While we no longer have a global env register, if PC is within
> +       the code_gen_buffer then we know that env is within a known register
> +       there, and we could have the signal handler extract that value.  */
> +    CPUArchState *env = cpu_single_env;

This just makes env a useless variable. The original code was trying
to restore the global variable, but the functions called later do not
use global env.

I'd change user-exec.c to work without global env use.

> +#else
>     if (cpu_single_env) {
> -        env = cpu_single_env; /* XXX: find a correct solution for multithread */
> +        env = cpu_single_env;
>     }
> +#endif
> +
>  #if defined(DEBUG_SIGNAL)
>     qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
>                 pc, address, is_write, *(unsigned long *)old_set);
> --
> 1.7.7.6
>
Richard Henderson - March 26, 2012, 4:31 p.m.
On 03/26/12 09:26, Blue Swirl wrote:
>> +#ifndef CONFIG_TCG_PASS_AREG0
>>     env = env1;
>> +#endif
> 
> Shouldn't longjmp() restore global registers as well? Actually, we
> return to cpu-exec.c which does not use global env. Isn't this
> useless?

Possibly.  I didn't think to try to actually remove these uses,
just get the code to compile without env being declared.

> I'd change user-exec.c to work without global env use.

I'll give it a shot...


r~

Patch

diff --git a/dyngen-exec.h b/dyngen-exec.h
index cfeef99..65fcb43 100644
--- a/dyngen-exec.h
+++ b/dyngen-exec.h
@@ -19,6 +19,10 @@ 
 #if !defined(__DYNGEN_EXEC_H__)
 #define __DYNGEN_EXEC_H__
 
+/* If the target has indicated that it does not need an AREG0,
+   don't declare the env variable at all, much less as a register.  */
+#if !defined(CONFIG_TCG_PASS_AREG0)
+
 #if defined(CONFIG_TCG_INTERPRETER)
 /* The TCG interpreter does not need a special register AREG0,
  * but it is possible to use one by defining AREG0.
@@ -65,4 +69,5 @@  register CPUArchState *env asm(AREG0);
 extern CPUArchState *env;
 #endif
 
+#endif /* !CONFIG_TCG_PASS_AREG0 */
 #endif /* !defined(__DYNGEN_EXEC_H__) */
diff --git a/user-exec.c b/user-exec.c
index cd905ff..e326104 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -58,7 +58,9 @@  void cpu_resume_from_signal(CPUArchState *env1, void *puc)
     struct sigcontext *uc = puc;
 #endif
 
+#ifndef CONFIG_TCG_PASS_AREG0
     env = env1;
+#endif
 
     /* XXX: restore cpu registers saved in host registers */
 
@@ -74,8 +76,8 @@  void cpu_resume_from_signal(CPUArchState *env1, void *puc)
         sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
     }
-    env->exception_index = -1;
-    longjmp(env->jmp_env, 1);
+    env1->exception_index = -1;
+    longjmp(env1->jmp_env, 1);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is
@@ -89,9 +91,18 @@  static inline int handle_cpu_signal(unsigned long pc, unsigned long address,
     TranslationBlock *tb;
     int ret;
 
+    /* XXX: find a correct solution for multithread */
+#ifdef CONFIG_TCG_PASS_AREG0
+    /* ??? While we no longer have a global env register, if PC is within
+       the code_gen_buffer then we know that env is within a known register
+       there, and we could have the signal handler extract that value.  */
+    CPUArchState *env = cpu_single_env;
+#else
     if (cpu_single_env) {
-        env = cpu_single_env; /* XXX: find a correct solution for multithread */
+        env = cpu_single_env;
     }
+#endif
+
 #if defined(DEBUG_SIGNAL)
     qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
                 pc, address, is_write, *(unsigned long *)old_set);