Re: [Qemu-devel] [PATCH 7/8] target-arm: implement vsli.64, vsri.64

[Peter Maydell]

On 31 January 2011 18:06,  <address@hidden> wrote:
> From: Christophe Lyon <address@hidden>
>
> Signed-off-by: Christophe Lyon <address@hidden>
> ---
>  target-arm/translate.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 61d4c4c..9150242 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4700,7 +4700,16 @@ static int disas_neon_data_insn(CPUState * env, 
> DisasContext *s, uint32_t insn)
>                             tcg_gen_add_i64(cpu_V0, cpu_V0, cpu_V1);
>                         } else if (op == 4 || (op == 5 && u)) {
>                             /* Insert */
> -                            cpu_abort(env, "VS[LR]I.64 not implemented");
> +                            neon_load_reg64(cpu_V1, rd + pass);
> +                            uint64_t mask;
> +                            if (op == 4) {
> +                                mask = 0xffffffffffffffffull >> -shift;
> +                            } else {
> +                                mask = 0xffffffffffffffffull << shift;
> +                            }

If shift is 64 or -64 then the result of this shift is undefined (and for
an x86 host we get the wrong results for eg "vsri.64 q5,q5,64").

You want to add an
  if (shift < -63 || shift > 63) {
    mask = 0;
  } else ...

clause (compare the 32 bit case.)

> +                            tcg_gen_andi_i64(cpu_V0, cpu_V0, mask);

This AND is harmless but unnecessary (and not specified in the
ARM ARM.)

-- PMM