From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O91bD-0000Di-0g for qemu-devel@nongnu.org; Mon, 03 May 2010 15:46:39 -0400 Received: from [140.186.70.92] (port=36349 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O91bB-0000C9-4M for qemu-devel@nongnu.org; Mon, 03 May 2010 15:46:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O91b9-00019l-3F for qemu-devel@nongnu.org; Mon, 03 May 2010 15:46:36 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:43043) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O91b8-00019a-Uj for qemu-devel@nongnu.org; Mon, 03 May 2010 15:46:35 -0400 Received: by fxm12 with SMTP id 12so2501420fxm.4 for ; Mon, 03 May 2010 12:46:33 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: Date: Mon, 3 May 2010 23:46:32 +0400 Message-ID: From: Igor Kovalenko Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: sparc64 lazy conditional codes evaluation List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel On Mon, May 3, 2010 at 11:24 PM, Blue Swirl wrote: > On 5/3/10, Igor Kovalenko wrote: >> Hi! >> >> =A0There is an issue with lazy conditional codes evaluation where >> =A0we return from trap handler with mismatching conditionals. >> >> =A0I seldom reproduce it here when dragging qemu window while >> =A0machine is working through silo initialization. I use gentoo minimal = cd >> =A0install-sparc64-minimal-20100322.iso but I think anything with silo b= oot >> =A0would experience the same. Once in a while it would report crc error, >> =A0unable to open cd partition or it would fail to decompress image. > > I think I've also seen this. > >> =A0Pattern that fails appears to require a sequence of compare insn >> =A0possibly followed by a few instructions which do not touch conditiona= ls, >> =A0then conditional branch insn. If it happens that we trap while proces= sing >> =A0conditional branch insn so it is restarted after return from trap the= n >> =A0seldom conditional codes are calculated incorrectly. >> >> =A0I cannot point to exact cause but it appears that after trap return >> =A0we may have CC_OP and CC_SRC* mismatch somewhere, >> =A0since adding more cond evaluation flushes over the code helps. >> >> =A0We already tried doing flush more frequently and it is still not >> =A0complete, so the question is how to finally do this once and right :) >> >> =A0Obviously I do not get the design of lazy evaluation right, but >> =A0the following list appears to be good start. Plan is to prepare >> =A0a change to qemu and find a way to test it. >> >> =A01. Since SPARC* is a RISC CPU it seems to be not profitable to >> =A0 =A0use DisasContext->cc_op to predict if flags should be not evaluat= ed >> =A0 =A0due to overriding insn. Instead we can drop cc_op from disassembl= er >> =A0 =A0context and simplify code to only use cc_op from env. > > Not currently, but in the future we may use that to do even lazier > flags computation. For example the sequence 'cmp x, y; bne target' > could be much more optimal by changing the branch to do the > comparison. Here's an old unfinished patch to do some of this. > >> =A0 =A0Another point is that we always write to env->cc_op when >> =A0translating *cc insns >> =A0 =A0This should solve any issue with dc->cc_op prediction going >> =A0 =A0out of sync with env->cc_op and cpu_cc_src* > > I think this is what is happening now. > >> =A02. We must flush lazy evaluation back to CC_OP_FLAGS in a few cases w= hen >> =A0 =A0a. conditional code is required by insn (like addc, cond branch e= tc.) >> =A0 =A0 =A0 - here we can optimize by evaluating specific bits (carry?) >> =A0 =A0 =A0 - not sure if it works in case we have two cond consuming in= sns, >> =A0 =A0 =A0 =A0 where first needs carry another needs the rest of flags > > Here's another patch to optimize C flag handling. It doesn't pass my > tests though. > >> =A0 =A0b. CCR is read by rdccr (helper_rdccr) >> =A0 =A0 =A0 - have to compute all flags >> =A0 =A0c. trap occurs and we prepare trap level context (saving pstate) >> =A0 =A0 =A0 - have to compute all flags >> =A0 =A0d. control goes out of tcg runtime (so gdbstub reads correct valu= e from env) >> =A0 =A0 =A0 - have to compute all flags > > Fully agree. Cool Still I'd propose to kill dc->cc_op, find a reliable way to test it and then add it back possibly with more optimizations. I'm lost in the code up to the point where I believe we need to save/restore cc_op and cpu_cc* while switching trap levels. --=20 Kind regards, Igor V. Kovalenko