* [Qemu-devel] [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) @ 2010-04-08 13:38 Stefan Weil 2010-04-08 22:44 ` [Qemu-devel] " Aurelien Jarno 0 siblings, 1 reply; 8+ messages in thread From: Stefan Weil @ 2010-04-08 13:38 UTC (permalink / raw) To: QEMU Developers; +Cc: Aurelien Jarno 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(-) 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-08 13:38 [Qemu-devel] [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) Stefan Weil @ 2010-04-08 22:44 ` Aurelien Jarno 2010-04-08 23:44 ` Paul Brook 2010-04-09 6:42 ` Stefan Weil 0 siblings, 2 replies; 8+ messages in thread From: Aurelien Jarno @ 2010-04-08 22:44 UTC (permalink / raw) To: Stefan Weil; +Cc: QEMU Developers 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 > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-08 22:44 ` [Qemu-devel] " Aurelien Jarno @ 2010-04-08 23:44 ` Paul Brook 2010-04-09 6:42 ` Stefan Weil 1 sibling, 0 replies; 8+ messages in thread From: Paul Brook @ 2010-04-08 23:44 UTC (permalink / raw) To: qemu-devel; +Cc: Aurelien Jarno > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-08 22:44 ` [Qemu-devel] " Aurelien Jarno 2010-04-08 23:44 ` Paul Brook @ 2010-04-09 6:42 ` Stefan Weil 2010-04-09 7:36 ` [Qemu-devel] " Stefan Weil 1 sibling, 1 reply; 8+ messages in thread From: Stefan Weil @ 2010-04-09 6:42 UTC (permalink / raw) To: Aurelien Jarno; +Cc: QEMU Developers 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-09 6:42 ` Stefan Weil @ 2010-04-09 7:36 ` Stefan Weil 2010-04-09 14:45 ` Richard Henderson 0 siblings, 1 reply; 8+ messages in thread From: Stefan Weil @ 2010-04-09 7:36 UTC (permalink / raw) To: QEMU Developers; +Cc: Aurelien Jarno 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 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). 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. Cc: Aurelien Jarno <aurelien@aurel32.net> Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- dyngen-exec.h | 2 +- tcg/mips/tcg-target.c | 6 ++++-- tcg/mips/tcg-target.h | 2 +- 3 files changed, 6 insertions(+), 4 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.c b/tcg/mips/tcg-target.c index f4fb615..c9094ee 100644 --- a/tcg/mips/tcg-target.c +++ b/tcg/mips/tcg-target.c @@ -1450,7 +1450,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 +1461,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 */ }; @@ -1524,6 +1525,7 @@ void tcg_target_init(TCGContext *s) tcg_regset_set_reg(s->reserved_regs, TCG_REG_AT); /* internal use */ tcg_regset_set_reg(s->reserved_regs, TCG_REG_T0); /* internal use */ tcg_regset_set_reg(s->reserved_regs, TCG_REG_RA); /* return address */ + tcg_regset_set_reg(s->reserved_regs, TCG_REG_FP); /* frame pointer */ tcg_regset_set_reg(s->reserved_regs, TCG_REG_SP); /* stack pointer */ tcg_add_target_add_op_defs(mips_op_defs); 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-09 7:36 ` [Qemu-devel] " Stefan Weil @ 2010-04-09 14:45 ` Richard Henderson 2010-04-09 15:28 ` Stefan Weil 0 siblings, 1 reply; 8+ messages in thread From: Richard Henderson @ 2010-04-09 14:45 UTC (permalink / raw) To: Stefan Weil; +Cc: QEMU Developers, Aurelien Jarno On 04/09/2010 12:36 AM, Stefan Weil wrote: > * Add fp to list of reserved registers. This is unnecessary. The register is not a frame pointer within tcg generated code. It's available for use as a call-saved register. r~ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-09 14:45 ` Richard Henderson @ 2010-04-09 15:28 ` Stefan Weil 2010-04-09 15:57 ` [Qemu-devel] " Aurelien Jarno 0 siblings, 1 reply; 8+ messages in thread From: Stefan Weil @ 2010-04-09 15:28 UTC (permalink / raw) To: QEMU Developers; +Cc: Aurelien Jarno, Richard Henderson 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 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) 2010-04-09 15:28 ` Stefan Weil @ 2010-04-09 15:57 ` Aurelien Jarno 0 siblings, 0 replies; 8+ messages in thread From: Aurelien Jarno @ 2010-04-09 15:57 UTC (permalink / raw) To: Stefan Weil; +Cc: QEMU Developers, Richard Henderson 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 > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-04-09 21:38 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-08 13:38 [Qemu-devel] [PATCH] tcp/mips: Change TCG_AREG0 (fp -> s0) Stefan Weil 2010-04-08 22:44 ` [Qemu-devel] " Aurelien Jarno 2010-04-08 23:44 ` Paul Brook 2010-04-09 6:42 ` Stefan Weil 2010-04-09 7:36 ` [Qemu-devel] " Stefan Weil 2010-04-09 14:45 ` Richard Henderson 2010-04-09 15:28 ` Stefan Weil 2010-04-09 15:57 ` [Qemu-devel] " Aurelien Jarno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).