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

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

Posted by Anton Blanchard 6 months, 2 weeks ago

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.

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 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

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

Posted by Alistair Francis 5 months, 1 week ago

On Thu, May 1, 2025 at 9:44 PM Anton Blanchard <antonb@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.
>
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>

Thanks!

Applied to riscv-to-apply.next

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
>
>

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

Posted by Alistair Francis 5 months, 1 week ago

On Thu, May 1, 2025 at 9:44 PM Anton Blanchard <antonb@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.
>
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

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
>
>