[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/12] target-arm: fix neon vsri, vshl and vsli
From: |
Laurent Desnogues |
Subject: |
Re: [Qemu-devel] [PATCH 07/12] target-arm: fix neon vsri, vshl and vsli ops |
Date: |
Wed, 21 Oct 2009 12:46:38 +0200 |
On Wed, Oct 21, 2009 at 12:17 PM, <address@hidden> wrote:
> Shift immediate value is incorrectly overwritten by a temporary
> variable in the processing of NEON vsri, vshl and vsli instructions.
>
> Signed-off-by: Juha Riihimäki <address@hidden>
> ---
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 59bf7bc..c92ecc6 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4094,7 +4094,7 @@ static int disas_neon_data_insn(CPUState * env,
> DisasContext *s, uint32_t insn)
> int pairwise;
> int u;
> int n;
> - uint32_t imm;
> + uint32_t imm, imm2;
I would prefer mask to imm2, but that's personal taste :-)
> TCGv tmp;
> TCGv tmp2;
> TCGv tmp3;
> @@ -4624,31 +4624,31 @@ static int disas_neon_data_insn(CPUState *
> env, DisasContext *s, uint32_t insn)
> switch (size) {
> case 0:
> if (op == 4)
> - imm = 0xff >> -shift;
> + imm2 = 0xff >> -shift;
> else
> - imm = (uint8_t)(0xff << shift);
> - imm |= imm << 8;
> - imm |= imm << 16;
> + imm2 = (uint8_t)(0xff << shift);
> + imm2 |= imm2 << 8;
> + imm2 |= imm2 << 16;
> break;
> case 1:
> if (op == 4)
> - imm = 0xffff >> -shift;
> + imm2 = 0xffff >> -shift;
> else
> - imm = (uint16_t)(0xffff << shift);
> - imm |= imm << 16;
> + imm2 = (uint16_t)(0xffff << shift);
> + imm2 |= imm2 << 16;
> break;
> case 2:
> if (op == 4)
> - imm = 0xffffffffu >> -shift;
> + imm2 = 0xffffffffu >> -shift;
> else
> - imm = 0xffffffffu << shift;
> + imm2 = 0xffffffffu << shift;
> break;
> default:
> abort();
> }
> tmp2 = neon_load_reg(rd, pass);
> - tcg_gen_andi_i32(tmp, tmp, imm);
> - tcg_gen_andi_i32(tmp2, tmp2, ~imm);
> + tcg_gen_andi_i32(tmp, tmp, imm2);
> + tcg_gen_andi_i32(tmp2, tmp2, ~imm2);
> tcg_gen_or_i32(tmp, tmp, tmp2);
> dead_tmp(tmp2);
> }
I mostly agree with this, except there's a mistake already
present in the original code: for a size of 2 the shift amount
can be 32 (look at VSRI in the ARM Architecture Reference
Manual). Note in this case, shift will be -32.
Laurent