Re: [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands

[Richard Henderson]

On 10/27/2016 02:43 PM, Laurent Vivier wrote:
> +static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
>  {
> -    TCGv reg, s32;
> -    TCGv_i64 t64, s64;
>      int logical = insn & 8;
> +    int left = insn & 0x100;
> +    int bits = opsize_bytes(opsize) * 8;
> +    TCGv reg = gen_extend(DREG(insn, 0), opsize, !logical);
> +    TCGv s32;
> +    TCGv_i64 t64, s64;
> +    TCGv zero;
>
> -    reg = DREG(insn, 0);
>      t64 = tcg_temp_new_i64();
>      s64 = tcg_temp_new_i64();
>      s32 = tcg_temp_new();
> @@ -2402,44 +2420,144 @@ DISAS_INSN(shift_reg)
>      tcg_gen_andi_i32(s32, DREG(insn, 9), 63);
>      tcg_gen_extu_i32_i64(s64, s32);
>
> -    /* Non-arithmetic shift clears V.  Use it as a source zero here.  */
> -    tcg_gen_movi_i32(QREG_CC_V, 0);
> +    zero = tcg_const_i32(0);
> +    tcg_gen_mov_i32(QREG_CC_V, zero);
>
> -    if (insn & 0x100) {
> -        tcg_gen_extu_i32_i64(t64, reg);
> +    tcg_gen_extu_i32_i64(t64, reg);
> +    if (left) {
> +        tcg_gen_shli_i64(t64, t64, 32 - bits);
>          tcg_gen_shl_i64(t64, t64, s64);
>          tcg_temp_free_i64(s64);
>          tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
>          tcg_temp_free_i64(t64);
> +        tcg_gen_sari_i32(QREG_CC_N, QREG_CC_N, 32 - bits);
>          tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 1);
> +
> +        /* Note that ColdFire always clears V,
> +           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)) {
> +            TCGv t0 = tcg_temp_new();
> +            TCGv t1 = tcg_temp_new();
> +            TCGv t2 = tcg_const_i32(bits);
> +
> +            tcg_gen_sub_i32(t2, t2, s32); /* t2 = bits - count */
> +
> +            tcg_gen_sari_i32(t0, reg, 31);
> +            tcg_gen_sar_i32(t1, reg, t2);
> +            tcg_temp_free(t2);
> +            tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, t0, t1);
> +            tcg_temp_free(t1);
> +
> +            /* if shift count >= bits, V is (reg != 0) */
> +            tcg_gen_setcond_i32(TCG_COND_NE, t0, reg, zero);
> +            tcg_gen_movcond_i32(TCG_COND_GE, QREG_CC_V, s32, t2, t0, 
> QREG_CC_V);
> +
> +            tcg_temp_free(t0);
> +
> +            tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
> +
> +            /* if shift count is zero, V is 0 */
> +            tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_V, s32, zero,
> +                                QREG_CC_V, zero);

This computation is not quite right, in that you compute bits - count and don't 
bound that against 0..31 for the sar_i32.

I think we can also to better for the two things you're special-casing: count 
>= bits and count == 0.  I need to think about that some more.


r~