Re: [Qemu-devel] [PATCH v3 06/16] target-m68k: add FPCR and FPSR

[Richard Henderson]

On 02/07/2017 11:59 AM, Laurent Vivier wrote:
>  void HELPER(itrunc_FP0)(CPUM68KState *env)
>  {
>      floatx80 res;
>
> +    set_float_rounding_mode(float_round_to_zero, &env->fp_status);
>      res = floatx80_round_to_int(FP0_to_floatx80(env), &env->fp_status);
> +    restore_rounding_mode(env);

It would be better to save/restore the current rounding mode as opposed to 
recomputing from the fpcr.

>  void HELPER(cmp_FP0_FP1)(CPUM68KState *env)
>  {
>      floatx80 fp0 = FP0_to_floatx80(env);
>      floatx80 fp1 = FP1_to_floatx80(env);
> -    floatx80 res;
> +    int float_compare;
>
> -    res = floatx80_sub(fp0, fp1, &env->fp_status);
> -    if (floatx80_is_quiet_nan(res, &env->fp_status)) {
> -        /* +/-inf compares equal against itself, but sub returns nan.  */
> -        if (!floatx80_is_quiet_nan(fp0, &env->fp_status)
> -            && !floatx80_is_quiet_nan(fp1, &env->fp_status)) {
> -            res = floatx80_zero;
> -            if (floatx80_lt_quiet(fp0, res, &env->fp_status)) {
> -                res = floatx80_chs(res);
> -            }
> -        }
> -    }
> -
> -    floatx80_to_FP0(env, res);
> +    float_compare = floatx80_compare(fp1, fp0, &env->fp_status);
> +    env->fpsr = (env->fpsr & ~FPSR_CC_MASK) | float_comp_to_cc(float_compare);
>  }
>
> -uint32_t HELPER(compare_FP0)(CPUM68KState *env)
> +void HELPER(tst_FP0)(CPUM68KState *env)
>  {
> -    floatx80 fp0 = FP0_to_floatx80(env);
> -    return floatx80_compare_quiet(fp0, floatx80_zero, &env->fp_status);
> +    uint32_t fpsr = 0;
> +    floatx80 val = FP0_to_floatx80(env);
> +
> +    if (floatx80_is_neg(val)) {
> +        fpsr |= FPSR_CC_N;
> +    }
> +
> +    if (floatx80_is_any_nan(val)) {
> +        fpsr |= FPSR_CC_A;
> +    } else if (floatx80_is_infinity(val)) {
> +        fpsr |= FPSR_CC_I;
> +    } else if (floatx80_is_zero(val)) {
> +        fpsr |= FPSR_CC_Z;
> +    }
> +    env->fpsr = (env->fpsr & ~FPSR_CC_MASK) | fpsr;
>  }

It would be better to pass in the old FPSR value, returning the new FPSR value, 
so that the helper can be marked TCG_CALL_NO_RWG -- not reading or modifying 
TCG globals.

> +        gen_helper_tst_FP0(cpu_env);

Making this, e.g. gen_helper_tst_FP0(QREG_FPSR, cpu_env).  Which will also 
happen to leave the FPSR in a host register where it can be used if the next 
insn is a fp branch.


r~