From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:56908)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1UZlQy-00023U-MG
	for qemu-devel@nongnu.org; Tue, 07 May 2013 13:12:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1UZlQt-0004Wl-Nk
	for qemu-devel@nongnu.org; Tue, 07 May 2013 13:12:12 -0400
Message-ID: <518935E4.70908@suse.de>
Date: Tue, 07 May 2013 19:12:04 +0200
From: Alexander Graf <agraf@suse.de>
MIME-Version: 1.0
References: <86sj20rql4.fsf@shell.gmplib.org> <5187ECAD.4050901@suse.de>
	<86obcorn76.fsf@shell.gmplib.org>
	<15FCEEAE-FE2D-44B9-9DC3-5419B29D5B16@suse.de>
	<86a9o7qe3u.fsf_-_@shell.gmplib.org>
	<86fvxypyru.fsf_-_@shell.gmplib.org>
In-Reply-To: <86fvxypyru.fsf_-_@shell.gmplib.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Qemu-ppc] Incorrect handling of more PPC64 insns
	(PATCH)
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: Torbjorn Granlund <tg@gmplib.org>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Richard Henderson <rth@twiddle.net>

On 05/07/2013 05:58 PM, Torbjorn Granlund wrote:
> OK, so took to reading some of translate to see how well it agrees with
> the PPC architecture definition.
>
> I spotted a bug with cmp, which was repeated 4 times.  Somebody decided
> that NARROW_MODE should affect the handling of cmp instructions, which
> is contrary to the ISA documentation.
>
> The first hunk is just a comment about suspicious code.  I don't suggest
> to apply that.
>
> Incidentally, this patch makes GMP testing go a bit further, and the
> testcase bug-qemu-ppc-again.s works correctly.

Richard, could you please proof read this?


Thanks!

Alex

>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 1a84653..c44b96d 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -665,6 +665,7 @@ static inline void gen_op_cmpi32(TCGv arg0, target_ulong arg1, int s, int crf)
>
>   static inline void gen_set_Rc0(DisasContext *ctx, TCGv reg)
>   {
> +// suspicious code -- tege
>       if (NARROW_MODE(ctx)) {
>           gen_op_cmpi32(reg, 0, 1, 0);
>       } else {
> @@ -675,7 +676,7 @@ static inline void gen_set_Rc0(DisasContext *ctx, TCGv reg)
>   /* cmp */
>   static void gen_cmp(DisasContext *ctx)
>   {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode&  0x00200000)) {
> +    if (!(ctx->opcode&  0x00200000)) {
>           gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                        1, crfD(ctx->opcode));
>       } else {
> @@ -687,7 +688,7 @@ static void gen_cmp(DisasContext *ctx)
>   /* cmpi */
>   static void gen_cmpi(DisasContext *ctx)
>   {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode&  0x00200000)) {
> +    if (!(ctx->opcode&  0x00200000)) {
>           gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
>                         1, crfD(ctx->opcode));
>       } else {
> @@ -699,7 +700,7 @@ static void gen_cmpi(DisasContext *ctx)
>   /* cmpl */
>   static void gen_cmpl(DisasContext *ctx)
>   {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode&  0x00200000)) {
> +    if (!(ctx->opcode&  0x00200000)) {
>           gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                        0, crfD(ctx->opcode));
>       } else {
> @@ -711,7 +712,7 @@ static void gen_cmpl(DisasContext *ctx)
>   /* cmpli */
>   static void gen_cmpli(DisasContext *ctx)
>   {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode&  0x00200000)) {
> +    if (!(ctx->opcode&  0x00200000)) {
>           gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
>                         0, crfD(ctx->opcode));
>       } else {
>