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

login
register
mail settings
Submitter Stefan Weil
Date April 8, 2010, 1:38 p.m.
Message ID <1270733932-7906-1-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/49731/
State New
Headers show

Comments

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

Register s0 was 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).

Cc: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 dyngen-exec.h         |    2 +-
 tcg/mips/tcg-target.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Aurelien Jarno - April 8, 2010, 10:44 p.m.
On Thu, Apr 08, 2010 at 03:38:52PM +0200, Stefan Weil wrote:
> Register fp is a bad choice for compilations without
> optimisation, because the compiler makes heavy use
> of this register (so the resulting code crashes).

I don't fully understand why the compiler makes use of this register in 
code where env is declared as register fp.

> Register s0 was 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).

The patch is not complete, at least some changes are missing to 
tcg_target_callee_save_regs.

> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  dyngen-exec.h         |    2 +-
>  tcg/mips/tcg-target.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/dyngen-exec.h b/dyngen-exec.h
> index b9f6969..85a2616 100644
> --- a/dyngen-exec.h
> +++ b/dyngen-exec.h
> @@ -61,7 +61,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.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.5.6.5
> 
>
Paul Brook - April 8, 2010, 11:44 p.m.
> On Thu, Apr 08, 2010 at 03:38:52PM +0200, Stefan Weil wrote:
> > Register fp is a bad choice for compilations without
> > optimisation, because the compiler makes heavy use
> > of this register (so the resulting code crashes).
> 
> I don't fully understand why the compiler makes use of this register in
> code where env is declared as register fp.

The frame pointer (and certain other registers, typically stack pointer, link 
register, PIC base, TLS base, etc.) are special. They have predefined uses, 
usually specified by the relevant platform ABI.

Defining a global register variable (or using -ffixed-reg) only excludes these 
registers from general register allocation.  It does not prevent them being 
used for specific purposes.  The user is responsible for ensuring that any 
assumptions/requirements made by the ABI are still met.

In some cases gcc can diagnose invalid uses. In others the overlap may be 
legitimate, but not necessarily what we intended in qemu. 

Paul
Stefan Weil - April 9, 2010, 6:42 a.m.
Aurelien Jarno schrieb:
> On Thu, Apr 08, 2010 at 03:38:52PM +0200, Stefan Weil wrote:
>   
>> Register fp is a bad choice for compilations without
>> optimisation, because the compiler makes heavy use
>> of this register (so the resulting code crashes).
>>     
>
> I don't fully understand why the compiler makes use of this register in 
> code where env is declared as register fp.
>   

fp = frame pointer is special. See Paul's answer.

>   
>> Register s0 was 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).
>>     
>
> The patch is not complete, at least some changes are missing to 
> tcg_target_callee_save_regs.
>   

Thanks. I'll send an updated patch.

Regards, Stefan

Patch

diff --git a/dyngen-exec.h b/dyngen-exec.h
index b9f6969..85a2616 100644
--- a/dyngen-exec.h
+++ b/dyngen-exec.h
@@ -61,7 +61,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.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