Re: [PATCH] target/m68k: Map FPU exceptions to FPSR register

[Richard Henderson <richard.henderson@linaro.org>]

On 8/2/23 17:55, Keith Packard via wrote:
> Add helpers for reading/writing the 68881 FPSR register so that
> changes in floating point exception state can be seen by the
> application.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>   target/m68k/cpu.h        |  2 ++
>   target/m68k/fpu_helper.c | 72 ++++++++++++++++++++++++++++++++++++++++
>   target/m68k/helper.c     |  4 +--
>   target/m68k/helper.h     |  2 ++
>   target/m68k/translate.c  |  4 +--
>   5 files changed, 80 insertions(+), 4 deletions(-)

Good catch.  Mostly ok.

> +static inline int cpu_m68k_exceptbits_from_host(int host_bits)
...
> +static inline int cpu_m68k_exceptbits_to_host(int target_bits)

No need for inline markers.

> +uint32_t HELPER(get_fpsr)(CPUM68KState *env)
> +{
> +    int host_flags = get_float_exception_flags(&env->fp_status);
> +    int target_flags = cpu_m68k_exceptbits_from_host(host_flags);
> +    int except = (env->fpsr & ~(0xf8)) | target_flags;
> +    return except;
> +}
> +
> +uint32_t cpu_m68k_get_fpsr(CPUM68KState *env)
> +{
> +    return HELPER(get_fpsr)(env);
> +}

In general it is bad form to call HELPER(foo) directly.
In this case it doesn't hurt, but better form to reverse the implementations.

> +void HELPER(set_fpsr)(CPUM68KState *env, uint32_t val)
> +{
> +    env->fpsr = val;
> +
> +    int host_flags = cpu_m68k_exceptbits_to_host((int) env->fpsr);
> +    set_float_exception_flags(host_flags, &env->fp_status);
> +}
> +
> +void cpu_m68k_set_fpsr(CPUM68KState *env, uint32_t val)
> +{
> +    return HELPER(set_fpsr)(env, val);
> +}

Likewise.

What's missing is an update to vmstate, to make sure all the architectural bits are 
properly saved. Add

     env->fpsr = cpu_m68k_get_fpsr(env);

to a (new) fpu_pre_save and

     cpu_m68k_set_fpsr(env, env->fpsr);

to fpu_post_load in target/m68k/cpu.c.


r~