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

[Laurent Vivier]

There is another bug on this one.

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
> +static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
...
> +        /* M68000 sets V if the most significant bit is changed at
> +         * any time during the shift operation.  Do this via creating
> +         * an extension of the sign bit, comparing, and discarding
> +         * the bits below the sign bit.  I.e.
> +         *     int64_t s = (intN_t)reg;
> +         *     int64_t t = (int64_t)(intN_t)reg << count;
> +         *     V = ((s ^ t) & (-1 << (bits - 1))) != 0
> +         */
> +        if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +            /* Sign extend the input to 64 bits; re-do the shift.  */
> +            tcg_gen_ext_i32_i64(t64, reg);
> +            tcg_gen_shl_i64(s64, t64, s64);
> +            /* Clear all bits that are unchanged.  */
> +            tcg_gen_xor_i64(t64, t64, s64);
> +            /* Ignore the bits below the sign bit.  */
> +            tcg_gen_andi_i64(t64, t64, -1ULL << (bits - 1));
> +            /* If any bits remain set, we have overflow.  */
> +            tcg_gen_setcondi_i64(TCG_COND_NE, t64, t64, 0);
> +            tcg_gen_extrl_i64_i32(QREG_CC_V, t64);
> +            tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);

if s64 is greater than 32, we lose all the bits needed to compute the V
flag. I think we can just add this to fix the problem:

         if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+            TCGv_i64 tt = tcg_const_i64(32);
+            /* if shift is greater than 32, use 32 */
+            tcg_gen_movcond_i64(TCG_COND_GT, s64, s64, tt, tt, s64);
+            tcg_temp_free_i64(tt);
             /* Sign extend the input to 64 bits; re-do the shift.  */
             tcg_gen_ext_i32_i64(t64, reg);
             tcg_gen_shl_i64(s64, t64, s64);

Is it correct?

Laurent