Re: [Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use

[Aurelien Jarno]

On Sun, May 22, 2011 at 07:55:53PM +0300, Blue Swirl wrote:
> On Sun, May 22, 2011 at 2:18 PM, Blue Swirl <address@hidden> wrote:
> > Make functions take a parameter for CPUState instead of relying
> > on global env. Pass CPUState pointer to TCG prologue, which moves
> > it to AREG0.
> 
> I found the problem with this patch on i386, TCG assumes that its
> caller is also using global env. Now i386 also works.
> 
> Updated patch attached, but here's the diff to previous version:
> 
> diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
> index 294fc7a..7248520 100644
> --- a/tcg/hppa/tcg-target.c
> +++ b/tcg/hppa/tcg-target.c
> @@ -1596,7 +1596,7 @@ static int tcg_target_callee_save_regs[] = {
>      TCG_REG_R14,
>      TCG_REG_R15,
>      TCG_REG_R16,
> -    /* R17 is the global env, so no need to save.  */
> +    TCG_REG_R17, /* R17 is the global env.  */
>      TCG_REG_R18
>  };
> 
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index ba031ab..72b3a48 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -1901,10 +1901,10 @@ static int tcg_target_callee_save_regs[] = {
>      TCG_REG_RBX,
>      TCG_REG_R12,
>      TCG_REG_R13,
> -    /* TCG_REG_R14, */ /* Currently used for the global env. */
> +    TCG_REG_R14, /* Currently used for the global env. */
>      TCG_REG_R15,
>  #else
> -    /* TCG_REG_EBP, */ /* Currently used for the global env. */
> +    TCG_REG_EBP, /* Currently used for the global env. */
>      TCG_REG_EBX,
>      TCG_REG_ESI,
>      TCG_REG_EDI,
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index a6b2457..cb2ab8b 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -1452,9 +1452,7 @@ static const TCGTargetOpDef mips_op_defs[] = {
>  };
> 
>  static int tcg_target_callee_save_regs[] = {
> -#if 0 /* used for the global env (TCG_AREG0), so no need to save */
> -    TCG_REG_S0,
> -#endif
> +    TCG_REG_S0,       /* used for the global env (TCG_AREG0) */
>      TCG_REG_S1,
>      TCG_REG_S2,
>      TCG_REG_S3,
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index dd2a85a..266e699 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -160,8 +160,7 @@ static const int tcg_target_callee_save_regs[] = {
>      TCG_REG_R24,
>      TCG_REG_R25,
>      TCG_REG_R26,
> -    /* TCG_REG_R27, */ /* currently used for the global env, so no
> -                          need to save */
> +    TCG_REG_R27, /* currently used for the global env */
>      TCG_REG_R28,
>      TCG_REG_R29,
>      TCG_REG_R30,
> diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
> index 83fa903..2e3cd2b 100644
> --- a/tcg/ppc64/tcg-target.c
> +++ b/tcg/ppc64/tcg-target.c
> @@ -151,8 +151,7 @@ static const int tcg_target_callee_save_regs[] = {
>      TCG_REG_R24,
>      TCG_REG_R25,
>      TCG_REG_R26,
> -    /* TCG_REG_R27, */ /* currently used for the global env, so no
> -                          need to save */
> +    TCG_REG_R27, /* currently used for the global env */
>      TCG_REG_R28,
>      TCG_REG_R29,
>      TCG_REG_R30,
> 
> For ARM, the handcrafted instructions below need to be changed to save also 
> r7:
>     /* stmdb sp!, { r4 - r6, r8 - r11, lr } */
>     tcg_out32(s, (COND_AL << 28) | 0x092d4f70);
> 
>     /* ldmia sp!, { r4 - r6, r8 - r11, pc } */
>     tcg_out32(s, (COND_AL << 28) | 0x08bd8f70);
> 
> ia64 doesn't look like saving anything. Sparc is OK with the 'save'.
> 

ia64 is indeed not correct, TCG_AREG0 is currently r7, and thus need to
be saved in the prologue and restore in the epilogue. This is something
to be added, given the current code simply doesn't use register which
need to be preserved (we have a bunch of register on this architecture).

I will work on a patch for ia64 when I finished with my current patches.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net