Re: [Qemu-devel] [PATCH 09/15] tcg-sparc: Do not use a global register for AREG0.

[Blue Swirl]

On Sun, Mar 25, 2012 at 22:27, Richard Henderson <address@hidden> wrote:
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  dyngen-exec.h |   20 +++++++++++---------
>  exec.c        |   16 ++++++++++++++--
>  2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/dyngen-exec.h b/dyngen-exec.h
> index 65fcb43..d673f9f 100644
> --- a/dyngen-exec.h
> +++ b/dyngen-exec.h
> @@ -41,13 +41,8 @@
>  #elif defined(__mips__)
>  #define AREG0 "s0"
>  #elif defined(__sparc__)
> -#ifdef CONFIG_SOLARIS
> -#define AREG0 "g2"
> -#elif HOST_LONG_BITS == 64
> -#define AREG0 "g5"
> -#else
> -#define AREG0 "g6"
> -#endif
> +/* Don't use a global register.  Working around glibc clobbering these
> +   global registers is more trouble than just using TLS.  */
>  #elif defined(__s390__)
>  #define AREG0 "r10"
>  #elif defined(__alpha__)
> @@ -62,12 +57,19 @@
>  #error unsupported CPU
>  #endif
>
> -#if defined(AREG0)
> +#ifdef AREG0
>  register CPUArchState *env asm(AREG0);
>  #else
> -/* TODO: Try env = cpu_single_env. */
> +/* It's tempting to #define env cpu_single_cpu, but that runs afoul of
> +   the other macro usage in target-foo/helper.h.  Instead use an alias.
> +   That has to happen where cpu_single_cpu is defined, so just a
> +   declaration here.  */
> +#ifdef __linux__
> +extern __thread CPUArchState *env;
> +#else
>  extern CPUArchState *env;
>  #endif
> +#endif /* AREG0 */
>
>  #endif /* !CONFIG_TCG_PASS_AREG0 */
>  #endif /* !defined(__DYNGEN_EXEC_H__) */
> diff --git a/exec.c b/exec.c
> index 6731ab8..d84caa5 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -124,9 +124,21 @@ static MemoryRegion io_mem_subpage_ram;
>  #endif
>
>  CPUArchState *first_cpu;
> -/* current CPU in the current thread. It is only valid inside
> -   cpu_exec() */
> +
> +/* Current CPU in the current thread. It is only valid inside cpu_exec().  */
>  DEFINE_TLS(CPUArchState *,cpu_single_env);
> +
> +/* In dyngen-exec.h, without AREG0, we fall back to an alias to 
> cpu_single_env.
> +   We can't actually tell from here whether that's needed or not, but it does
> +   not hurt to go ahead and make the declaration.  */
> +#ifndef CONFIG_TCG_PASS_AREG0
> +extern
> +#ifdef __linux__
> +  __thread
> +#endif
> +  CPUArchState *env __attribute__((alias("tls__cpu_single_env")));
> +#endif /* CONFIG_TCG_PASS_AREG0 */

Please use DECLARE_TLS/DEFINE_TLS and global env accesses should also
use tls_var().

> +
>  /* 0 = Do not count executed instructions.
>    1 = Precise instruction counting.
>    2 = Adaptive rate instruction counting.  */
> --
> 1.7.7.6
>