From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41833)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1Wuh6x-000091-Dm
	for qemu-devel@nongnu.org; Wed, 11 Jun 2014 07:54:41 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1Wuh6r-0006ri-2m
	for qemu-devel@nongnu.org; Wed, 11 Jun 2014 07:54:35 -0400
Message-ID: <5398436F.8040409@gmail.com>
Date: Wed, 11 Jun 2014 06:54:23 -0500
From: Tom Musta <tommusta@gmail.com>
MIME-Version: 1.0
References: <CA+mWYFvkiGGexDEUpD9KXiBYFVWQAXDjVODeX=miM9udQtObDw@mail.gmail.com>
	<CA+mWYFuNTpEG8Q4A87A6B8A0u968=LX_2qogJh2U-S6vKa4CoQ@mail.gmail.com>
In-Reply-To: <CA+mWYFuNTpEG8Q4A87A6B8A0u968=LX_2qogJh2U-S6vKa4CoQ@mail.gmail.com>
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: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Sorav Bansal <sbansal@cse.iitd.ernet.in>, qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org

On 6/11/2014 3:01 AM, Sorav Bansal wrote:
> [I had posted to qemu-ppc list earlier, reposting to qemu-devel also
> on being suggested to do so. Also adding some description about the
> patch]
> 
> Hi,
> 
> I saw a minor bug in the gen_mcrxr() function in
> target-ppc/translate.c. Here is the patch for your consideration. I
> have verified the patch by checking the generated code using a
> SAT-solver based verification tool.
> 
> thanks,
> Sorav
> 
> Patch Description:
> As I see it, the XER[SO], XER[OV], and XER[CA] flags are stored in the
> least significant bit (bit 0) of their respective registers. They need
> to be shifted left (by their respective offsets) to generate the final
> XER value. The old code seemed to be assuming that  the flags are
> stored in bit 2, and was shifting them right (by their respective
> offsets), which seems incorrect.
> 
> From 31f39e258cbb289c2e0a3c3adde87cde7ae02a15 Mon Sep 17 00:00:00 2001
> From: Sorav Bansal <sbansal@cse.iitd.ernet.in>
> Date: Tue, 10 Jun 2014 19:01:12 +0530
> Subject: [PATCH] Fix to the translation of mcrxr instruction
> 
> ---
>  target-ppc/translate.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index f089014..b513998 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4147,8 +4147,8 @@ 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(dst, dst, 2);
> +    tcg_gen_shli_i32(t1, t1, 1);
>      tcg_gen_or_i32(dst, dst, t0);
>      tcg_gen_or_i32(dst, dst, t1);
>      tcg_temp_free_i32(t0);
> --
> 1.7.9.5
> 

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.