Re: [Qemu-devel] [PATCH] SH4: convert fmov/fadd to TCG

["Blue Swirl" <blauwirbel@gmail.com>]

On 8/30/08, Shin-ichiro KAWASAKI <kawasaki@juno.dti.ne.jp> wrote:
> This patch converts two SH4 float instructions, 'fmov Rm,Rn' and
>  'fadd' into TCG.  Before converting other float instructions
>  into TCG, comments on it will be appreciated.
>
>  - TCG variables intorudced for float operation : FT[01], and DT[01].
>  - I think float registers 'fregs' are not to be mapped for
>   TCG variables, because TCG does not support float operations, now.

In TCG README Fabrice mentioned that he's working on a floating point
version. It would be nice to get that committed, already m68k has a
fake version. I did not use TCG variables for float just because I've
been waiting for that. It's not possible to pass env->fp_status
cleanly now (for example to call softfloat functions directly). And
for example on Sparc host, moving data from integer registers to
floating point registers must use memory in between them so there the
integer TCG registers should be separate from float TCG registers.

>  +uint32_t helper_fadd_FT(uint32_t t0, uint32_t t1, CPUState * env)
>  +{
>  +    float32 ret = float32_add(*(float32*)&t0, *(float32*)&t1, &env->fp_status);
>  +    return *(uint32_t*)(&ret);
>  +}

There is no need to pass env as a parameter, it's available to
op_helper.c code anyway.

>  +static inline void gen_ld_frN(TCGv ft, TCGv cpu_env, uint32_t reg)
>  +{
>  +    tcg_gen_ld_i32(ft, cpu_env, offsetof(CPUState, fregs[reg]));
>  +}
>  +
>  +static inline void gen_ld_drN(TCGv dt, TCGv cpu_env, uint32_t reg)
>  +{
>  +    TCGv tmp = tcg_temp_new(TCG_TYPE_I64);
>  +
>  +    tcg_gen_ld_i64(dt, cpu_env, offsetof(CPUState, fregs[reg]));
>  +    tcg_gen_shli_i64(tmp, dt, 32);
>  +    tcg_gen_shri_i64(dt, dt, 32);
>  +    tcg_gen_or_i64(dt, tmp, dt);
>  +
>  +    tcg_temp_free(tmp);
>  +}

I guess that like in Sparc, two 32 bit fregs can be combined to a 64
bit double. Please have a look at target-sparc/translate.c on the use
of CPU_DoubleU, there should be a safer way to access the registers.

Otherwise, good work!