Re: [PATCH] target/riscv: Fix fcvt.s.bf16 NaN box checking

[Alistair Francis]

On Wed, Apr 30, 2025 at 6:23 PM Anton Blanchard <ant...@tenstorrent.com> wrote:
>
> fcvt.s.bf16 uses the FP16 check_nanbox_h() which returns an FP16
> quiet NaN. Add check_nanbox_bf16() which returns a BF16 quiet NaN.

You are missing a Signed-off-by line

Alistair

> ---
>  target/riscv/fpu_helper.c |  2 +-
>  target/riscv/internals.h  | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index 91b1a56d10..31c17399fc 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -756,6 +756,6 @@ uint64_t helper_fcvt_bf16_s(CPURISCVState *env, uint64_t 
> rs1)
>
>  uint64_t helper_fcvt_s_bf16(CPURISCVState *env, uint64_t rs1)
>  {
> -    float16 frs1 = check_nanbox_h(env, rs1);
> +    float16 frs1 = check_nanbox_bf16(env, rs1);
>      return nanbox_s(env, bfloat16_to_float32(frs1, &env->fp_status));
>  }
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index 213aff31d8..794c81bf7c 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -142,6 +142,22 @@ static inline float16 check_nanbox_h(CPURISCVState *env, 
> uint64_t f)
>      }
>  }
>
> +static inline float16 check_nanbox_bf16(CPURISCVState *env, uint64_t f)
> +{
> +    /* Disable nanbox check when enable zfinx */
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
> +        return (uint16_t)f;
> +    }
> +
> +    uint64_t mask = MAKE_64BIT_MASK(16, 48);
> +
> +    if (likely((f & mask) == mask)) {
> +        return (uint16_t)f;
> +    } else {
> +        return 0x7FC0u; /* default qnan */
> +    }
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>  /* Our implementation of SysemuCPUOps::has_work */
>  bool riscv_cpu_has_work(CPUState *cs);
> --
> 2.34.1
>
>