[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 2/2] target-ppc: mcrfs should always update FEX/VX
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH 2/2] target-ppc: mcrfs should always update FEX/VX and only clear exception bits |
Date: |
Fri, 29 Jan 2016 14:17:17 +1100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Sun, Jan 24, 2016 at 03:41:26PM +0000, James Clarke wrote:
> Signed-off-by: James Clarke <address@hidden>
So, first, for a patch making a subtle behavioural change like this a
detailed commit message is absolutely essential. In this case I can
take the description from 0/2, but in future please include
rationale's like that in the individual patches, so they'll appear in
the git history without extra work on my part.
But.. there's a more serious bug here, so I've backed this out of
ppc-for-2.6...
[snip]
> @@ -2501,17 +2501,24 @@ static void gen_mcrfs(DisasContext *ctx)
> {
> TCGv tmp = tcg_temp_new();
> int bfa;
> + int nibble;
> + int shift;
>
> if (unlikely(!ctx->fpu_enabled)) {
> gen_exception(ctx, POWERPC_EXCP_FPU);
> return;
> }
> - bfa = 4 * (7 - crfS(ctx->opcode));
> - tcg_gen_shri_tl(tmp, cpu_fpscr, bfa);
> + bfa = crfS(ctx->opcode);
> + nibble = 7 - bfa;
> + shift = 4 * nibble;
> + tcg_gen_shri_tl(tmp, cpu_fpscr, shift);
> tcg_gen_trunc_tl_i32(cpu_crf[crfD(ctx->opcode)], tmp);
> - tcg_temp_free(tmp);
> tcg_gen_andi_i32(cpu_crf[crfD(ctx->opcode)], cpu_crf[crfD(ctx->opcode)],
> 0xf);
> - tcg_gen_andi_tl(cpu_fpscr, cpu_fpscr, ~(0xF << bfa));
> + /* Only the exception bits (including FX) should be cleared if read */
> + tcg_gen_andi_tl(tmp, cpu_fpscr, ~((0xF << shift) & FP_EX_CLEAR_BITS));
> + /* FEX and VX need to be updated, so don't set fpscr directly */
> + gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble);
This doesn't compile. For 64-bit targets we get:
CC ppc64-softmmu/target-ppc/translate.o
/home/dwg/src/qemu/target-ppc/translate.c: In function ‘gen_mcrfs’:
/home/dwg/src/qemu/target-ppc/translate.c:2520:42: error: passing argument 3 of
‘gen_helper_store_fpscr’ makes pointe
r from integer without a cast [-Werror=int-conversion]
gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble);
^
In file included from /home/dwg/src/qemu/include/exec/helper-gen.h:59:0,
from /home/dwg/src/qemu/tcg/tcg-op.h:27,
from /home/dwg/src/qemu/target-ppc/translate.c:23:
/home/dwg/src/qemu/target-ppc/helper.h:56:58: note: expected ‘TCGv_i32 {aka
struct TCGv_i32_d *}’ but argument is of
type ‘int’
For 32-bit targets it's worse:
CC ppcemb-softmmu/target-ppc/translate.o
/home/dwg/src/qemu/target-ppc/translate.c: In function ‘gen_mcrfs’:
/home/dwg/src/qemu/target-ppc/translate.c:2520:37: error: passing argument 2 of
‘gen_helper_store_fpscr’ from incompa
tible pointer type [-Werror=incompatible-pointer-types]
gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble);
^
In file included from /home/dwg/src/qemu/include/exec/helper-gen.h:59:0,
from /home/dwg/src/qemu/tcg/tcg-op.h:27,
from /home/dwg/src/qemu/target-ppc/translate.c:23:
/home/dwg/src/qemu/target-ppc/helper.h:56:58: note: expected ‘TCGv_i64 {aka
struct TCGv_i64_d *}’ but argument is of
type ‘TCGv_i32 {aka struct TCGv_i32_d *}’
/home/dwg/src/qemu/target-ppc/translate.c:2520:42: error: passing argument 3 of
‘gen_helper_store_fpscr’ makes pointe
r from integer without a cast [-Werror=int-conversion]
gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble);
^
In file included from /home/dwg/src/qemu/include/exec/helper-gen.h:59:0,
from /home/dwg/src/qemu/tcg/tcg-op.h:27,
from /home/dwg/src/qemu/target-ppc/translate.c:23:
/home/dwg/src/qemu/target-ppc/helper.h:56:58: note: expected ‘TCGv_i32 {aka
struct TCGv_i32_d *}’ but argument is of
type ‘int’
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature