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

[Richard Henderson]

On 11/27/2016 11:30 AM, Laurent Vivier wrote:
> 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);

Hmm.  I guess the test case is input like 8 << 63, where the sign bit is clear,
the input is non-zero, and we shift out all of the set bits.

I can't think of anything cleaner than your movcond solution.


r~