From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50186) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOdC1-0005zX-P9 for qemu-devel@nongnu.org; Mon, 23 May 2011 18:01:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QOdC0-0006yg-Bq for qemu-devel@nongnu.org; Mon, 23 May 2011 18:01:41 -0400 Received: from hall.aurel32.net ([88.191.126.93]:51891) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOdC0-0006yc-4J for qemu-devel@nongnu.org; Mon, 23 May 2011 18:01:40 -0400 Date: Tue, 24 May 2011 00:01:37 +0200 From: Aurelien Jarno Message-ID: <20110523220137.GB28505@volta.aurel32.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel On Sun, May 22, 2011 at 07:55:53PM +0300, Blue Swirl wrote: > On Sun, May 22, 2011 at 2:18 PM, Blue Swirl 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 aurelien@aurel32.net http://www.aurel32.net