From patchwork Tue May 8 18:32:41 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Qemu-ppc,for-1.1,3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode Date: Tue, 08 May 2012 08:32:41 -0000 From: Alexander Graf X-Patchwork-Id: 157780 Message-Id: <4487E0EF-5383-425C-8C4B-D5C91BA742D8@suse.de> To: =?iso-8859-1?Q?Andreas_F=E4rber?= Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 08.05.2012, at 20:20, Alexander Graf wrote: > > On 08.05.2012, at 19:43, Alexander Graf wrote: > >> >> On 08.05.2012, at 19:39, Alexander Graf wrote: >> >>> >>> On 07.05.2012, at 01:46, Andreas Färber wrote: >>> >>>> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3. >>>> Automate the register numbering to avoid double-coding the two modes, >>>> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4 >>>> but not for Darwin ABI. >>>> >>>> Based on patch by malc. >>> >>> AREG0-free PPC works for me with this patch on a ppc32 host. >>> >>> Tested-by: Alexander Graf >> >> I take that one back - it breaks once things get more complex. Debugging ... > > I have no idea how this code could have ever worked. We are getting unknown register numbers as input variables. Then mr them into our C ABI parameter registers (r3+). Then we call the C helper to do the load/store for us. > > Now, what if one of those input parameters is within r3-r7 (which is the highest register passed into the C ld function)? We'd happily do something like > > mr r3, r5 > mr r4, r3 > mr r5, ... > > at which point we have long overwritten the actual value of r3! This actually makes me wonder. How do we guarantee that all the volatile registers at the state of a ld/st are declared as clobbered? [a few minutes later] Aha! Through constraints. Then the below patch (which also fixes the issue for me) is a lot better of course: diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c index ace5548..5750a2b 100644 --- a/tcg/ppc/tcg-target.c +++ b/tcg/ppc/tcg-target.c @@ -258,6 +258,9 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str) tcg_regset_set32(ct->u.regs, 0, 0xffffffff); tcg_regset_reset_reg(ct->u.regs, TCG_REG_R3); tcg_regset_reset_reg(ct->u.regs, TCG_REG_R4); + tcg_regset_reset_reg(ct->u.regs, TCG_REG_R5); + tcg_regset_reset_reg(ct->u.regs, TCG_REG_R6); + tcg_regset_reset_reg(ct->u.regs, TCG_REG_R7); break; case 'K': /* qemu_st[8..32] constraint */ ct->ct |= TCG_CT_REG; @@ -267,6 +270,7 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str) tcg_regset_reset_reg(ct->u.regs, TCG_REG_R5); #if TARGET_LONG_BITS == 64 tcg_regset_reset_reg(ct->u.regs, TCG_REG_R6); + tcg_regset_reset_reg(ct->u.regs, TCG_REG_R7); #endif break; case 'M': /* qemu_st64 constraint */