Patchwork tcp/mips: Change TCG_AREG0 (fp -> s0)

login
register
mail settings
Submitter Stefan Weil
Date April 9, 2010, 3:28 p.m.
Message ID <1270826920-590-1-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/49849/
State New
Headers show

Comments

Stefan Weil - April 9, 2010, 3:28 p.m.
Register fp (frame pointer) is a bad choice for compilations
without optimisation, because the compiler makes heavy use
of this register (so the resulting code crashes).

Register s0 had been used for TCG_AREG1 in earlier releases,
but was no longer used and is now free for TCG_AREG0.

The resulting code works for compilations without
optimisation (tested with qemu mips in qemu mips
on x86 host).

v2:

* Remove s0 from tcg_target_callee_save_regs and add fp there.
  Hint from Aurelien Jarno, thanks.

* Add fp to list of reserved registers.

v3:

* Don't add fp to list of reserved registers
  Thanks to Richard Henderson.

* Remove s0 from tcg_target_reg_alloc_order.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 dyngen-exec.h         |    2 +-
 tcg/mips/tcg-target.c |    7 +++++--
 tcg/mips/tcg-target.h |    2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)
Aurelien Jarno - April 9, 2010, 3:57 p.m.
On Fri, Apr 09, 2010 at 05:28:40PM +0200, Stefan Weil wrote:
> Register fp (frame pointer) is a bad choice for compilations
> without optimisation, because the compiler makes heavy use
> of this register (so the resulting code crashes).
> 
> Register s0 had been used for TCG_AREG1 in earlier releases,
> but was no longer used and is now free for TCG_AREG0.
> 
> The resulting code works for compilations without
> optimisation (tested with qemu mips in qemu mips
> on x86 host).
> 
> v2:
> 
> * Remove s0 from tcg_target_callee_save_regs and add fp there.
>   Hint from Aurelien Jarno, thanks.
> 
> * Add fp to list of reserved registers.
> 
> v3:
> 
> * Don't add fp to list of reserved registers
>   Thanks to Richard Henderson.
> 
> * Remove s0 from tcg_target_reg_alloc_order.
> 
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  dyngen-exec.h         |    2 +-
>  tcg/mips/tcg-target.c |    7 +++++--
>  tcg/mips/tcg-target.h |    2 +-
>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/dyngen-exec.h b/dyngen-exec.h
> index d04eda8..0700a2d 100644
> --- a/dyngen-exec.h
> +++ b/dyngen-exec.h
> @@ -59,7 +59,7 @@ extern int printf(const char *, ...);
>  #elif defined(__hppa__)
>  #define AREG0 "r17"
>  #elif defined(__mips__)
> -#define AREG0 "fp"
> +#define AREG0 "s0"
>  #elif defined(__sparc__)
>  #ifdef CONFIG_SOLARIS
>  #define AREG0 "g2"
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index f4fb615..31296fc 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -69,7 +69,9 @@ static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
>  
>  /* check if we really need so many registers :P */
>  static const int tcg_target_reg_alloc_order[] = {
> +#if 0 /* used for the global env (TCG_AREG0), so don't use it here */
>      TCG_REG_S0,
> +#endif

This is not something necessary. The fact this register is declared as
TCG_AREG0, will make TCG skip this automatically.

>      TCG_REG_S1,
>      TCG_REG_S2,
>      TCG_REG_S3,
> @@ -1450,7 +1452,9 @@ 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_S1,
>      TCG_REG_S2,
>      TCG_REG_S3,
> @@ -1459,8 +1463,7 @@ static int tcg_target_callee_save_regs[] = {
>      TCG_REG_S6,
>      TCG_REG_S7,
>      TCG_REG_GP,
> -    /* TCG_REG_FP, */ /* currently used for the global env, so np
> -                         need to save */
> +    TCG_REG_FP,
>      TCG_REG_RA,       /* should be last for ABI compliance */
>  };
>  
> diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
> index 0292d33..0028bfa 100644
> --- a/tcg/mips/tcg-target.h
> +++ b/tcg/mips/tcg-target.h
> @@ -97,7 +97,7 @@ enum {
>  #undef TCG_TARGET_HAS_ext16u_i32   /* andi rt, rs, 0xffff */
>  
>  /* Note: must be synced with dyngen-exec.h */
> -#define TCG_AREG0 TCG_REG_FP
> +#define TCG_AREG0 TCG_REG_S0
>  
>  /* guest base is supported */
>  #define TCG_TARGET_HAS_GUEST_BASE
> -- 
> 1.7.0
> 
>

Patch

diff --git a/dyngen-exec.h b/dyngen-exec.h
index d04eda8..0700a2d 100644
--- a/dyngen-exec.h
+++ b/dyngen-exec.h
@@ -59,7 +59,7 @@  extern int printf(const char *, ...);
 #elif defined(__hppa__)
 #define AREG0 "r17"
 #elif defined(__mips__)
-#define AREG0 "fp"
+#define AREG0 "s0"
 #elif defined(__sparc__)
 #ifdef CONFIG_SOLARIS
 #define AREG0 "g2"
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index f4fb615..31296fc 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -69,7 +69,9 @@  static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
 
 /* check if we really need so many registers :P */
 static const int tcg_target_reg_alloc_order[] = {
+#if 0 /* used for the global env (TCG_AREG0), so don't use it here */
     TCG_REG_S0,
+#endif
     TCG_REG_S1,
     TCG_REG_S2,
     TCG_REG_S3,
@@ -1450,7 +1452,9 @@  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_S1,
     TCG_REG_S2,
     TCG_REG_S3,
@@ -1459,8 +1463,7 @@  static int tcg_target_callee_save_regs[] = {
     TCG_REG_S6,
     TCG_REG_S7,
     TCG_REG_GP,
-    /* TCG_REG_FP, */ /* currently used for the global env, so np
-                         need to save */
+    TCG_REG_FP,
     TCG_REG_RA,       /* should be last for ABI compliance */
 };
 
diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index 0292d33..0028bfa 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -97,7 +97,7 @@  enum {
 #undef TCG_TARGET_HAS_ext16u_i32   /* andi rt, rs, 0xffff */
 
 /* Note: must be synced with dyngen-exec.h */
-#define TCG_AREG0 TCG_REG_FP
+#define TCG_AREG0 TCG_REG_S0
 
 /* guest base is supported */
 #define TCG_TARGET_HAS_GUEST_BASE