From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QF7cq-0006P1-TG for qemu-devel@nongnu.org; Wed, 27 Apr 2011 12:30:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QF7cp-0003SQ-5J for qemu-devel@nongnu.org; Wed, 27 Apr 2011 12:30:04 -0400 Received: from mail-qy0-f173.google.com ([209.85.216.173]:39285) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QF7cp-0003SL-1C for qemu-devel@nongnu.org; Wed, 27 Apr 2011 12:30:03 -0400 Received: by qyk36 with SMTP id 36so1997563qyk.4 for ; Wed, 27 Apr 2011 09:30:02 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20110425202927.GD21831@volta.aurel32.net> From: Artyom Tarasenko Date: Wed, 27 Apr 2011 18:29:42 +0200 Message-ID: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Laurent Desnogues , peter.maydell@linaro.org, qemu-devel , Aurelien Jarno On Tue, Apr 26, 2011 at 8:36 PM, Blue Swirl wrote: > On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko w= rote: >> On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno = wrote: >>> On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote: >>>> On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues >>>> wrote: >>>> > On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko >>>> > wrote: >>>> >> On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues >>>> >> wrote: >>>> >>> On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko wrote: >>>> >>>> On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko >>>> >>>> wrote: >>>> >>>>>>> Do you have public test case? >>>> >>>>>>> It is possible to code this delay slot write test but real iss= ue may >>>> >>>>>>> be corruption elsewhere. >>>> >>>> >>>> >>>> The test case is trivial: it's just the two instructions, branch = and wrpr. >>>> >>>> >>>> >>>>> In theory there could be multiple issues including compiler indu= ced ones. >>>> >>>>> I'd prefer to see some kind of reproducible testcase. >>>> >>>> >>>> >>>> Ok, attached a 40 byte long test (the first 32 bytes are not used= and >>>> >>>> needed only because the bios entry point is 0x20). >>>> >>>> >>>> >>>> $ git pull && make && sparc64-softmmu/qemu-system-sparc64 -bios >>>> >>>> test-wrpr.bin -nographic >>>> >>>> Already up-to-date. >>>> >>>> make[1]: Nothing to be done for `all'. >>>> >>>> /mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error >>>> >>>> Aborted >>>> >>> >>>> >>> The problem seems to be that wrpr is using a non-local >>>> >>> TCG tmp (cpu_tmp0). >>>> >> >>>> >> Just tried the test case with write to %pil - seems like write itse= lf is OK. >>>> >> The issue appears to be with save_state() call since adding save_st= ate >>>> >> to %pil case provokes the same tcg abort. >>>> > >>>> > The problem is that cpu_tmp0, not being a local tmp, doesn't >>>> > need to be saved across helper calls. =A0This results in the >>>> > TCG "optimizer" getting rid of it even though it's later used. >>>> > Look at the log and you'll see what I mean :-) >>>> >>>> I'm not very comfortable with tcg yet. Would it be possible to teach >>>> optimizer working with delay slots? Or do I look in the wrong place. >>>> >>> >>> The problem is not on the TCG side, but on the target-sparc/translate.c >>> side: >>> >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 0x32: /* wrwim, V9 wrpr *= / >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!supervis= or(dc)) >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto = priv_insn; >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tcg_gen_xor_t= l(cpu_tmp0, cpu_src1, cpu_src2); >>> |=A0#ifdef TARGET_SPARC64 >>> >>> Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not >>> saved across TCG branches. >>> >>> [...] >>> >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 case 6: // ps= tate >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 save_= state(dc, cpu_cond); >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 gen_h= elper_wrpstate(cpu_tmp0); >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dc->n= pc =3D DYNAMIC_PC; >>> | =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break= ; >>> >>> save_state() calls save_npc(), which in turns might call >>> gen_generic_branch(): >> >> Hmm. This is not the only instruction using save_state() and cpu_tmp0. >> At least ldd is another example. >> >>> |=A0static inline void gen_generic_branch(target_ulong npc1, target_ulo= ng npc2, >>> |=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0TCGv r_cond) >>> | { >>> | =A0 =A0 int l1, l2; >>> | >>> | =A0 =A0 l1 =3D gen_new_label(); >>> |=A0 =A0 =A0l2 =3D gen_new_label(); >>> | >>> | =A0 =A0 tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1); >>> | >>> | =A0 =A0 tcg_gen_movi_tl(cpu_npc, npc1); >>> | =A0 =A0 tcg_gen_br(l2); >>> | >>> | =A0 =A0 gen_set_label(l1); >>> |=A0 =A0 =A0tcg_gen_movi_tl(cpu_npc, npc2); >>> | =A0 =A0 gen_set_label(l2); >>> | } >>> >>> And here is the TCG branch, which drop the TCG temp cpu_temp0. >>> >>> The solution is either to rewrite gen_generic_branch() without TCG >>> branches, or to use a TCG temp local instead of a TCG temp. >> >> You mean something like >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 6: // pstate >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0T= CGv r_temp; >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0r= _temp =3D tcg_temp_new(); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0t= cg_gen_mov_tl(r_temp, cpu_tmp0); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0s= ave_state(dc, cpu_cond); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0g= en_helper_wrpstate(r_temp); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0t= cg_temp_free(r_temp); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0d= c->npc =3D DYNAMIC_PC; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; >> >> ? >> This fails with the same error message. > > Close, but you need to use tcg_temp_local_new(). Does this work? > > diff --git a/target-sparc/translate.c b/target-sparc/translate.c > index 3c958b2..52fa2f1 100644 > --- a/target-sparc/translate.c > +++ b/target-sparc/translate.c > @@ -3505,9 +3505,15 @@ static void disas_sparc_insn(DisasContext * dc) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tcg_gen_m= ov_tl(cpu_tbr, cpu_tmp0); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 case 6: // pstate > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0save_sta= te(dc, cpu_cond); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gen_help= er_wrpstate(cpu_tmp0); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dc->npc = =3D DYNAMIC_PC; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{ > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= TCGv r_tmp =3D tcg_temp_local_new(); > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= tcg_gen_mov_tl(r_tmp, cpu_tmp0); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= save_state(dc, cpu_cond); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= gen_helper_wrpstate(r_tmp); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= tcg_temp_free_ptr(r_tmp); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= dc->npc =3D DYNAMIC_PC; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 case 7: // tl > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 save_stat= e(dc, cpu_cond); > Yep. This (with tcg_temp_free) passes my tests, thanks! A bonus question: how is immediate case handled? I don't see any explicit "if (IS_IMM)" and yet it seems to produce the expected results for the immediate instructions. --=20 Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/