Re: [PATCH v2 16/22] target/loongarch: Add floating point move instruction translation

From: Richard Henderson <richard.henderson@linaro.org>
To: Song Gao <gaosong@loongson.cn>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, thuth@redhat.com, chenhuacai@gmail.com,
	philmd@redhat.com, yangxiaojuan@loongson.cn, laurent@vivier.eu,
	maobibo@loongson.cn, alistair.francis@wdc.com,
	pbonzini@redhat.com, alex.bennee@linaro.org
Subject: Re: [PATCH v2 16/22] target/loongarch: Add floating point move instruction translation
Date: Thu, 22 Jul 2021 20:29:05 -1000	[thread overview]
Message-ID: <643a7954-cf1c-78dd-2d08-f0969ea6127e@linaro.org> (raw)
In-Reply-To: <1626861198-6133-17-git-send-email-gaosong@loongson.cn>

On 7/20/21 11:53 PM, Song Gao wrote:
> This patch implement floationg point move instruction translation.
> 
> This includes:
> - FMOV.{S/D}
> - FSEL
> - MOVGR2FR.{W/D}, MOVGR2FRH.W
> - MOVFR2GR.{S/D}, MOVFRH2GR.S
> - MOVGR2FCSR, MOVFCSR2GR
> - MOVFR2CF, MOVCF2FR
> - MOVGR2CF, MOVCF2GR
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/fpu_helper.c |  80 +++++++++++++
>   target/loongarch/helper.h     |   6 +
>   target/loongarch/insns.decode |  41 +++++++
>   target/loongarch/trans.inc.c  | 270 ++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 397 insertions(+)
> 
> diff --git a/target/loongarch/fpu_helper.c b/target/loongarch/fpu_helper.c
> index 162085a..7662715 100644
> --- a/target/loongarch/fpu_helper.c
> +++ b/target/loongarch/fpu_helper.c
> @@ -379,6 +379,11 @@ uint64_t helper_fp_logb_d(CPULoongArchState *env, uint64_t fp)
>       return fp1;
>   }
>   
> +void helper_movreg2cf(CPULoongArchState *env, uint32_t cd, target_ulong src)
> +{
> +    env->active_fpu.cf[cd & 0x7] = src & 0x1;
> +}

tcg_gen_andi_tl + tcg_gen_st8_tl.

> +target_ulong helper_fsel(CPULoongArchState *env, target_ulong fj,
> +                         target_ulong fk, uint32_t ca)
> +{
> +    if (env->active_fpu.cf[ca & 0x7]) {
> +        return fk;
> +    } else {
> +        return fj;
> +    }
> +}

tcg_gen_movcond_i64.

> +void helper_movgr2fcsr(CPULoongArchState *env, target_ulong arg1,
> +                       uint32_t fcsr)
> +{
> +    switch (fcsr) {
> +    case 0:
> +        env->active_fpu.fcsr0 = arg1;
> +        break;
> +    case 1:
> +        env->active_fpu.fcsr0 = (arg1 & FCSR0_M1) |
> +                                (env->active_fpu.fcsr0 & ~FCSR0_M1);
> +        break;
> +    case 2:
> +        env->active_fpu.fcsr0 = (arg1 & FCSR0_M2) |
> +                                (env->active_fpu.fcsr0 & ~FCSR0_M2);
> +        break;
> +    case 3:
> +        env->active_fpu.fcsr0 = (arg1 & FCSR0_M3) |
> +                                (env->active_fpu.fcsr0 & ~FCSR0_M3);
> +        break;

This is easily implemented inline, followed by a single helper call to re-load the 
rounding mode (if required by the mask).

> +    case 16:
> +        env->active_fpu.vcsr16 = arg1;
> +        break;

The documentation I have does not describe the vector stuff?

> +    default:
> +        printf("%s: warning, fcsr '%d' not supported\n", __func__, fcsr);
> +        assert(0);
> +        break;

No printfs, no assert.  This should have been caught by

> +target_ulong helper_movcf2reg(CPULoongArchState *env, uint32_t cj)
> +{
> +    return (target_ulong)env->active_fpu.cf[cj & 0x7];
> +}

tcg_gen_ld8u_tl.

r~