Re: [Qemu-devel] [PATCH v2 3/5] target-m68k: Inline shifts

[Laurent Vivier]

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index 4f224d7..1b3765f 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -2883,48 +2883,205 @@ DISAS_INSN(addx_mem)
>      gen_store(s, opsize, addr_dest, QREG_CC_N);
>  }
>  
> -/* TODO: This could be implemented without helper functions.  */
> -DISAS_INSN(shift_im)
> +static inline void shift_im(DisasContext *s, uint16_t insn, int opsize)
>  {
> -    TCGv reg;
> -    int tmp;
> -    TCGv shift;
> +    int count = (insn >> 9) & 7;
> +    int logical = insn & 8;
> +    int left = insn & 0x100;
> +    int bits = opsize_bytes(opsize) * 8;
> +    TCGv reg = gen_extend(DREG(insn, 0), opsize, !logical);
> +
> +    if (count == 0) {
> +        count = 8;
> +    }
> +
> +    tcg_gen_movi_i32(QREG_CC_V, 0);
> +    if (left) {
> +        tcg_gen_shri_i32(QREG_CC_C, reg, bits - count);
> +        tcg_gen_shli_i32(QREG_CC_N, reg, count);
> +
> +        /* Note that ColdFire always clears V (done above),
> +           while M68000 sets if the most significant bit is changed at
> +           any time during the shift operation */
> +        if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +            /* if shift count >= bits, V is (reg != 0) */
> +            if (count >= bits) {
> +                tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, reg, QREG_CC_V);
> +            } else {
> +                TCGv t0 = tcg_temp_new();
> +                tcg_gen_sari_i32(QREG_CC_V, reg, bits - 1);
> +                tcg_gen_sari_i32(t0, t0, bits - count);

t0 is used unitialized, I think we should have here:

                tcg_gen_sari_i32(t0, reg, bits - count - 1);

moreover we must use "bits - count - 1" to use also the current most
significant bit to compute V flag.

> +                tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, QREG_CC_V, t0);
> +                tcg_temp_free(t0);
> +            }
> +            tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
> +        }
> +    } else {
> +        tcg_gen_shri_i32(QREG_CC_C, reg, count - 1);
> +        if (logical) {
> +            tcg_gen_shri_i32(QREG_CC_N, reg, count);
> +        } else {
> +            tcg_gen_sari_i32(QREG_CC_N, reg, count);
> +        }
> +    }

Thanks,
Laurent