Re: [Qemu-devel] [PATCH v5 07/17] target/m68k: add chk and chk2

[Laurent Vivier]

Le 02/01/2018 à 17:41, Richard Henderson a écrit :
> On 01/01/2018 05:10 PM, Laurent Vivier wrote:
>> +    SRC_EA(env, tsrc, opsize, 1, NULL);
>> +    src = tcg_temp_local_new();
>> +    tcg_gen_mov_i32(src, tsrc);
>> +
>> +    reg = tcg_temp_local_new();
>> +    gen_ext(reg, DREG(insn, 9), opsize, 1);
>> +    gen_flush_flags(s);
>> +    update_cc_op(s);
>> +
>> +    l1 = gen_new_label();
>> +    l2 = gen_new_label();
>> +    tcg_gen_brcondi_i32(TCG_COND_GE, reg, 0, l1);
>> +    tcg_gen_movi_i32(QREG_CC_N, -1);
>> +    tcg_gen_movi_i32(QREG_PC, s->pc);
>> +    gen_raise_exception(EXCP_CHK);
>> +    tcg_gen_br(l2);
> 
> Unreachable branch after exception.
> 
>> +    gen_set_label(l1);
>> +    tcg_gen_brcond_i32(TCG_COND_LE, reg, src, l2);
>> +    tcg_gen_movi_i32(QREG_CC_N, 0);
>> +    tcg_gen_movi_i32(QREG_PC, s->pc);
>> +    gen_raise_exception(EXCP_CHK);
>> +    gen_set_label(l2);
>> +    tcg_temp_free(src);
>> +    tcg_temp_free(reg);
> 
> Does real hardware not change flags at all if the chk passes?  The manual says

Real hardware (68040) always clears N if reg > 0.

> it is undefined, which would greatly simplify all this.  Especially just
> assigning reg to CC_N...
> 
> Generally I put conditional traps like this into a helper, which allows
> straight-line optimization of the non-trapping path to continue.  In this 
> case,
> something like
> 
> void HELPER(chk)(CPUM68kState *env, int32_t val, int32_t ub)
> {
>     if (val < 0 || val > ub) {
>         CPUState *cs = CPU(m68k_env_get_cpu(env));
> 
>       /* Recover PC and CC_OP for the beginning of the insn.  */
>         cpu_restore_state(cs, GETPC());
> 
>         /* Adjust PC and FLAGS to end of the insn.  */
>         env->pc += 2;
>         helper_flush_flags(env, env->cc_op);
>         env->cc_n = val;
> 
>         cs->exception_index = EXCP_CHK;
>         cpu_loop_exit(cs);
>     }
> }

Even with assigning reg directly to CC_N the use of an helper seems
better. I'm going to update the patch with your helper, and add one for
chk2 too.

Thanks,
Laurent