From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37397) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WunCX-0004oA-ML for qemu-devel@nongnu.org; Wed, 11 Jun 2014 14:24:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WunCR-0000D9-3U for qemu-devel@nongnu.org; Wed, 11 Jun 2014 14:24:45 -0400 Message-ID: <53989EDA.5000903@gmail.com> Date: Wed, 11 Jun 2014 13:24:26 -0500 From: Tom Musta MIME-Version: 1.0 References: <5398436F.8040409@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] Fwd: Patch: fix to gen_mcrxr() in target-ppc/translate.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sorav Bansal Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf On 6/11/2014 9:23 AM, Sorav Bansal wrote: >> Please read my comments again. I agree that SO, OV and CA are stored in the LSB of their internal QEMU representation. The problem is that, even with your patch, these bits are not being correctly shifted into the target four bit CR field. >> > > Ah, I forgot that PPC spec starts bit numbering at the MSB. Here is > the revised patch. > > > From da0a962a6d14fe699ebb7cc12450c7de9553b66a Mon Sep 17 00:00:00 2001 > From: Sorav Bansal > Date: Wed, 11 Jun 2014 19:49:49 +0530 > Subject: [PATCH] Fixed the translation of the mcrxr ppc instruction > > --- > target-ppc/translate.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index f089014..59a92b9 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -4147,8 +4147,9 @@ static void gen_mcrxr(DisasContext *ctx) > tcg_gen_trunc_tl_i32(t0, cpu_so); > tcg_gen_trunc_tl_i32(t1, cpu_ov); > tcg_gen_trunc_tl_i32(dst, cpu_ca); > - tcg_gen_shri_i32(t0, t0, 2); > - tcg_gen_shri_i32(t1, t1, 1); > + tcg_gen_shli_i32(t0, t0, 3); > + tcg_gen_shli_i32(t1, t1, 2); > + tcg_gen_shli_i32(dst, dst, 1); > tcg_gen_or_i32(dst, dst, t0); > tcg_gen_or_i32(dst, dst, t1); > tcg_temp_free_i32(t0); > Your patch is missing the signoff. Other than that, this does properly shift into the CR field. My other comments about using deposit and lack of handling XER[35] still stand, but I don't believe your patch makes things worse. Reviewed-by: Tom Musta