[PATCH 0/3] OpenRISC updates for user space FPU

[PATCH 0/3] OpenRISC updates for user space FPU

[Stafford Horne]

This series adds support for the FPU related architecture changes defined in
architecture spec revision v1.4.

 - https://openrisc.io/revisions/r1.4

In summary the architecture changes are:

 - Change FPCSR SPR permissions to allow for reading and writing from user
   space.
 - Clarify that FPU underflow detection is done by detecting tininess before
   rounding.

Previous to this series FPCSR reads and writes from user-mode in QEMU would
throw an illegal argument exception.  The proper behavior should have been to
treat these operations as no-ops as the cpu implementations do.  As mentioned
series changes FPCSR read/write to follow the spec.

The series has been tested with the FPU support added in glibc test suite and
all math tests are passing.

Stafford Horne (3):
  target/openrisc: Allow fpcsr access in user mode
  target/openrisc: Set PC to cpu state on FPU exception
  target/openrisc: Setup FPU for detecting tininess before rounding

 target/openrisc/cpu.c        |  5 +++
 target/openrisc/fpu_helper.c |  4 ++
 target/openrisc/sys_helper.c | 45 +++++++++++++++++-----
 target/openrisc/translate.c  | 72 ++++++++++++++++--------------------
 4 files changed, 76 insertions(+), 50 deletions(-)

-- 
2.39.1

[PATCH 1/3] target/openrisc: Allow fpcsr access in user mode

[Stafford Horne]

As per OpenRISC spec 1.4 FPCSR can be read and written in user mode.

Update mtspr and mfspr helpers to support this by moving the is_user
check into the helper.

There is a logic change here to no longer throw an illegal instruction
exception when executing mtspr/mfspr in user mode.  The illegal
instruction exception is not part of the spec, so this should be OK.

Link: https://raw.githubusercontent.com/openrisc/doc/master/openrisc-arch-1.4-rev0.pdf
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/sys_helper.c | 45 +++++++++++++++++-----
 target/openrisc/translate.c  | 72 ++++++++++++++++--------------------
 2 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index ec145960e3..8a0259c710 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -29,17 +29,37 @@
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
+static inline bool is_user(CPUOpenRISCState *env)
+{
+#ifdef CONFIG_USER_ONLY
+    return true;
+#else
+    return (env->sr & SR_SM) == 0;
+#endif
+}
+
 void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)
 {
-#ifndef CONFIG_USER_ONLY
     OpenRISCCPU *cpu = env_archcpu(env);
+#ifndef CONFIG_USER_ONLY
     CPUState *cs = env_cpu(env);
     target_ulong mr;
     int idx;
 #endif
 
+    /* Handle user accessible SPRs first.  */
     switch (spr) {
+    case TO_SPR(0, 20): /* FPCSR */
+        cpu_set_fpcsr(env, rb);
+        return;
+    }
+
+    if (is_user(env)) {
+        raise_exception(cpu, EXCP_ILLEGAL);
+    }
+
 #ifndef CONFIG_USER_ONLY
+    switch (spr) {
     case TO_SPR(0, 11): /* EVBAR */
         env->evbar = rb;
         break;
@@ -187,12 +207,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)
         cpu_openrisc_timer_update(cpu);
         qemu_mutex_unlock_iothread();
         break;
-#endif
-
-    case TO_SPR(0, 20): /* FPCSR */
-        cpu_set_fpcsr(env, rb);
-        break;
     }
+#endif
 }
 
 target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
@@ -204,10 +220,22 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
     OpenRISCCPU *cpu = env_archcpu(env);
     CPUState *cs = env_cpu(env);
     int idx;
+#else
+    OpenRISCCPU *cpu = env_archcpu(env);
 #endif
 
+    /* Handle user accessible SPRs first.  */
     switch (spr) {
+    case TO_SPR(0, 20): /* FPCSR */
+        return env->fpcsr;
+    }
+
+    if (is_user(env)) {
+        raise_exception(cpu, EXCP_ILLEGAL);
+    }
+
 #ifndef CONFIG_USER_ONLY
+    switch (spr) {
     case TO_SPR(0, 0): /* VR */
         return env->vr;
 
@@ -324,11 +352,8 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
         cpu_openrisc_count_update(cpu);
         qemu_mutex_unlock_iothread();
         return cpu_openrisc_count_get(cpu);
-#endif
-
-    case TO_SPR(0, 20): /* FPCSR */
-        return env->fpcsr;
     }
+#endif
 
     /* for rd is passed in, if rd unchanged, just keep it back.  */
     return rd;
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 76e53c78d4..43ba0cc1ad 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -819,45 +819,12 @@ static bool trans_l_xori(DisasContext *dc, arg_rri *a)
 
 static bool trans_l_mfspr(DisasContext *dc, arg_l_mfspr *a)
 {
-    check_r0_write(dc, a->d);
-
-    if (is_user(dc)) {
-        gen_illegal_exception(dc);
-    } else {
-        TCGv spr = tcg_temp_new();
-
-        if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-            gen_io_start();
-            if (dc->delayed_branch) {
-                tcg_gen_mov_tl(cpu_pc, jmp_pc);
-                tcg_gen_discard_tl(jmp_pc);
-            } else {
-                tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
-            }
-            dc->base.is_jmp = DISAS_EXIT;
-        }
+    TCGv spr = tcg_temp_new();
 
-        tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
-        gen_helper_mfspr(cpu_R(dc, a->d), cpu_env, cpu_R(dc, a->d), spr);
-    }
-    return true;
-}
-
-static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a)
-{
-    if (is_user(dc)) {
-        gen_illegal_exception(dc);
-    } else {
-        TCGv spr;
+    check_r0_write(dc, a->d);
 
-        if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-            gen_io_start();
-        }
-        /* For SR, we will need to exit the TB to recognize the new
-         * exception state.  For NPC, in theory this counts as a branch
-         * (although the SPR only exists for use by an ICE).  Save all
-         * of the cpu state first, allowing it to be overwritten.
-         */
+    if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
+        gen_io_start();
         if (dc->delayed_branch) {
             tcg_gen_mov_tl(cpu_pc, jmp_pc);
             tcg_gen_discard_tl(jmp_pc);
@@ -865,11 +832,36 @@ static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a)
             tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
         }
         dc->base.is_jmp = DISAS_EXIT;
+    }
+
+    tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
+    gen_helper_mfspr(cpu_R(dc, a->d), cpu_env, cpu_R(dc, a->d), spr);
+    return true;
+}
+
+static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a)
+{
+    TCGv spr = tcg_temp_new();
 
-        spr = tcg_temp_new();
-        tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
-        gen_helper_mtspr(cpu_env, spr, cpu_R(dc, a->b));
+    if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
+        gen_io_start();
     }
+    /*
+     * For SR, we will need to exit the TB to recognize the new
+     * exception state.  For NPC, in theory this counts as a branch
+     * (although the SPR only exists for use by an ICE).  Save all
+     * of the cpu state first, allowing it to be overwritten.
+     */
+    if (dc->delayed_branch) {
+        tcg_gen_mov_tl(cpu_pc, jmp_pc);
+        tcg_gen_discard_tl(jmp_pc);
+    } else {
+        tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
+    }
+    dc->base.is_jmp = DISAS_EXIT;
+
+    tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
+    gen_helper_mtspr(cpu_env, spr, cpu_R(dc, a->b));
     return true;
 }
 
-- 
2.39.1

[PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception

[Stafford Horne]

Store the PC to ensure the correct value can be read in the exception
handler.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/fpu_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/openrisc/fpu_helper.c b/target/openrisc/fpu_helper.c
index f9e34fa2cc..1feebb9ac7 100644
--- a/target/openrisc/fpu_helper.c
+++ b/target/openrisc/fpu_helper.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
+#include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 #include "exception.h"
 #include "fpu/softfloat.h"
@@ -55,6 +56,9 @@ void HELPER(update_fpcsr)(CPUOpenRISCState *env)
         if (tmp) {
             env->fpcsr |= tmp;
             if (env->fpcsr & FPCSR_FPEE) {
+                CPUState *cs = env_cpu(env);
+
+                cpu_restore_state(cs, GETPC());
                 helper_exception(env, EXCP_FPE);
             }
         }
-- 
2.39.1

[PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding

[Stafford Horne]

OpenRISC defines tininess to be detected before rounding.  Setup qemu to
obey this.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 0ce4f796fa..cdbff26fb5 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -22,6 +22,7 @@
 #include "qemu/qemu-print.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "fpu/softfloat-helpers.h"
 #include "tcg/tcg.h"
 
 static void openrisc_cpu_set_pc(CPUState *cs, vaddr value)
@@ -90,6 +91,10 @@ static void openrisc_cpu_reset_hold(Object *obj)
     s->exception_index = -1;
     cpu_set_fpcsr(&cpu->env, 0);
 
+    set_default_nan_mode(1, &cpu->env.fp_status);
+    set_float_detect_tininess(float_tininess_before_rounding,
+                              &cpu->env.fp_status);
+
 #ifndef CONFIG_USER_ONLY
     cpu->env.picmr = 0x00000000;
     cpu->env.picsr = 0x00000000;
-- 
2.39.1

Re: [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode

[Stafford Horne]

On Tue, May 02, 2023 at 07:57:29PM +0100, Stafford Horne wrote:
> As per OpenRISC spec 1.4 FPCSR can be read and written in user mode.
> 
> Update mtspr and mfspr helpers to support this by moving the is_user
> check into the helper.
> 
> There is a logic change here to no longer throw an illegal instruction
> exception when executing mtspr/mfspr in user mode.  The illegal
> instruction exception is not part of the spec, so this should be OK.

This is wrong, I considered doing it but left the exception in (moved to the
helper).  I will remove this bit of the commit messages in the next version.
But it is something we could consider doing.

Conversely, the architecture pec should be more clear as to what happens when
mfspr/mtspr privileges are violated.

-Stafford

> Link: https://raw.githubusercontent.com/openrisc/doc/master/openrisc-arch-1.4-rev0.pdf
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  target/openrisc/sys_helper.c | 45 +++++++++++++++++-----
>  target/openrisc/translate.c  | 72 ++++++++++++++++--------------------
>  2 files changed, 67 insertions(+), 50 deletions(-)
> 
> diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
> index ec145960e3..8a0259c710 100644
> --- a/target/openrisc/sys_helper.c
> +++ b/target/openrisc/sys_helper.c
> @@ -29,17 +29,37 @@
>  
>  #define TO_SPR(group, number) (((group) << 11) + (number))
>  
> +static inline bool is_user(CPUOpenRISCState *env)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return true;
> +#else
> +    return (env->sr & SR_SM) == 0;
> +#endif
> +}
> +
>  void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)
>  {
> -#ifndef CONFIG_USER_ONLY
>      OpenRISCCPU *cpu = env_archcpu(env);
> +#ifndef CONFIG_USER_ONLY
>      CPUState *cs = env_cpu(env);
>      target_ulong mr;
>      int idx;
>  #endif
>  
> +    /* Handle user accessible SPRs first.  */
>      switch (spr) {
> +    case TO_SPR(0, 20): /* FPCSR */
> +        cpu_set_fpcsr(env, rb);
> +        return;
> +    }
> +
> +    if (is_user(env)) {
> +        raise_exception(cpu, EXCP_ILLEGAL);
> +    }
> +
>  #ifndef CONFIG_USER_ONLY
> +    switch (spr) {
>      case TO_SPR(0, 11): /* EVBAR */
>          env->evbar = rb;
>          break;
> @@ -187,12 +207,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)
>          cpu_openrisc_timer_update(cpu);
>          qemu_mutex_unlock_iothread();
>          break;
> -#endif
> -
> -    case TO_SPR(0, 20): /* FPCSR */
> -        cpu_set_fpcsr(env, rb);
> -        break;
>      }
> +#endif
>  }
>  
>  target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
> @@ -204,10 +220,22 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
>      OpenRISCCPU *cpu = env_archcpu(env);
>      CPUState *cs = env_cpu(env);
>      int idx;
> +#else
> +    OpenRISCCPU *cpu = env_archcpu(env);
>  #endif
>  
> +    /* Handle user accessible SPRs first.  */
>      switch (spr) {
> +    case TO_SPR(0, 20): /* FPCSR */
> +        return env->fpcsr;
> +    }
> +
> +    if (is_user(env)) {
> +        raise_exception(cpu, EXCP_ILLEGAL);
> +    }
> +
>  #ifndef CONFIG_USER_ONLY
> +    switch (spr) {
>      case TO_SPR(0, 0): /* VR */
>          return env->vr;
>  
> @@ -324,11 +352,8 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
>          cpu_openrisc_count_update(cpu);
>          qemu_mutex_unlock_iothread();
>          return cpu_openrisc_count_get(cpu);
> -#endif
> -
> -    case TO_SPR(0, 20): /* FPCSR */
> -        return env->fpcsr;
>      }
> +#endif
>  
>      /* for rd is passed in, if rd unchanged, just keep it back.  */
>      return rd;
> diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
> index 76e53c78d4..43ba0cc1ad 100644
> --- a/target/openrisc/translate.c
> +++ b/target/openrisc/translate.c
> @@ -819,45 +819,12 @@ static bool trans_l_xori(DisasContext *dc, arg_rri *a)
>  
>  static bool trans_l_mfspr(DisasContext *dc, arg_l_mfspr *a)
>  {
> -    check_r0_write(dc, a->d);
> -
> -    if (is_user(dc)) {
> -        gen_illegal_exception(dc);
> -    } else {
> -        TCGv spr = tcg_temp_new();
> -
> -        if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
> -            gen_io_start();
> -            if (dc->delayed_branch) {
> -                tcg_gen_mov_tl(cpu_pc, jmp_pc);
> -                tcg_gen_discard_tl(jmp_pc);
> -            } else {
> -                tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
> -            }
> -            dc->base.is_jmp = DISAS_EXIT;
> -        }
> +    TCGv spr = tcg_temp_new();
>  
> -        tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
> -        gen_helper_mfspr(cpu_R(dc, a->d), cpu_env, cpu_R(dc, a->d), spr);
> -    }
> -    return true;
> -}
> -
> -static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a)
> -{
> -    if (is_user(dc)) {
> -        gen_illegal_exception(dc);
> -    } else {
> -        TCGv spr;
> +    check_r0_write(dc, a->d);
>  
> -        if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
> -            gen_io_start();
> -        }
> -        /* For SR, we will need to exit the TB to recognize the new
> -         * exception state.  For NPC, in theory this counts as a branch
> -         * (although the SPR only exists for use by an ICE).  Save all
> -         * of the cpu state first, allowing it to be overwritten.
> -         */
> +    if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
>          if (dc->delayed_branch) {
>              tcg_gen_mov_tl(cpu_pc, jmp_pc);
>              tcg_gen_discard_tl(jmp_pc);
> @@ -865,11 +832,36 @@ static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a)
>              tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
>          }
>          dc->base.is_jmp = DISAS_EXIT;
> +    }
> +
> +    tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
> +    gen_helper_mfspr(cpu_R(dc, a->d), cpu_env, cpu_R(dc, a->d), spr);
> +    return true;
> +}
> +
> +static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a)
> +{
> +    TCGv spr = tcg_temp_new();
>  
> -        spr = tcg_temp_new();
> -        tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
> -        gen_helper_mtspr(cpu_env, spr, cpu_R(dc, a->b));
> +    if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
>      }
> +    /*
> +     * For SR, we will need to exit the TB to recognize the new
> +     * exception state.  For NPC, in theory this counts as a branch
> +     * (although the SPR only exists for use by an ICE).  Save all
> +     * of the cpu state first, allowing it to be overwritten.
> +     */
> +    if (dc->delayed_branch) {
> +        tcg_gen_mov_tl(cpu_pc, jmp_pc);
> +        tcg_gen_discard_tl(jmp_pc);
> +    } else {
> +        tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
> +    }
> +    dc->base.is_jmp = DISAS_EXIT;
> +
> +    tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k);
> +    gen_helper_mtspr(cpu_env, spr, cpu_R(dc, a->b));
>      return true;
>  }
>  
> -- 
> 2.39.1
> 

Re: [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception

[Richard Henderson]

On 5/2/23 19:57, Stafford Horne wrote:
> @@ -55,6 +56,9 @@ void HELPER(update_fpcsr)(CPUOpenRISCState *env)
>           if (tmp) {
>               env->fpcsr |= tmp;
>               if (env->fpcsr & FPCSR_FPEE) {
> +                CPUState *cs = env_cpu(env);
> +
> +                cpu_restore_state(cs, GETPC());
>                   helper_exception(env, EXCP_FPE);

Better to mirror do_range().

r~

Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding

[Richard Henderson]

On 5/2/23 19:57, Stafford Horne wrote:
> OpenRISC defines tininess to be detected before rounding.  Setup qemu to
> obey this.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>   target/openrisc/cpu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index 0ce4f796fa..cdbff26fb5 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -22,6 +22,7 @@
>   #include "qemu/qemu-print.h"
>   #include "cpu.h"
>   #include "exec/exec-all.h"
> +#include "fpu/softfloat-helpers.h"
>   #include "tcg/tcg.h"
>   
>   static void openrisc_cpu_set_pc(CPUState *cs, vaddr value)
> @@ -90,6 +91,10 @@ static void openrisc_cpu_reset_hold(Object *obj)
>       s->exception_index = -1;
>       cpu_set_fpcsr(&cpu->env, 0);
>   
> +    set_default_nan_mode(1, &cpu->env.fp_status);
> +    set_float_detect_tininess(float_tininess_before_rounding,
> +                              &cpu->env.fp_status);

You don't mention the nan change in the commit message.


r~

Re: [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception

[Stafford Horne]

On Wed, May 03, 2023 at 08:36:13AM +0100, Richard Henderson wrote:
> On 5/2/23 19:57, Stafford Horne wrote:
> > @@ -55,6 +56,9 @@ void HELPER(update_fpcsr)(CPUOpenRISCState *env)
> >           if (tmp) {
> >               env->fpcsr |= tmp;
> >               if (env->fpcsr & FPCSR_FPEE) {
> > +                CPUState *cs = env_cpu(env);
> > +
> > +                cpu_restore_state(cs, GETPC());
> >                   helper_exception(env, EXCP_FPE);
> 
> Better to mirror do_range().

OK.

Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding

[Stafford Horne]

On Wed, May 03, 2023 at 08:37:31AM +0100, Richard Henderson wrote:
> On 5/2/23 19:57, Stafford Horne wrote:
> > OpenRISC defines tininess to be detected before rounding.  Setup qemu to
> > obey this.
> > 
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> >   target/openrisc/cpu.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> > index 0ce4f796fa..cdbff26fb5 100644
> > --- a/target/openrisc/cpu.c
> > +++ b/target/openrisc/cpu.c
> > @@ -22,6 +22,7 @@
> >   #include "qemu/qemu-print.h"
> >   #include "cpu.h"
> >   #include "exec/exec-all.h"
> > +#include "fpu/softfloat-helpers.h"
> >   #include "tcg/tcg.h"
> >   static void openrisc_cpu_set_pc(CPUState *cs, vaddr value)
> > @@ -90,6 +91,10 @@ static void openrisc_cpu_reset_hold(Object *obj)
> >       s->exception_index = -1;
> >       cpu_set_fpcsr(&cpu->env, 0);
> > +    set_default_nan_mode(1, &cpu->env.fp_status);
> > +    set_float_detect_tininess(float_tininess_before_rounding,
> > +                              &cpu->env.fp_status);
> 
> You don't mention the nan change in the commit message.

Right, and I am not sure I need it.  Let me remove it and run tests again.  I
was just adding it as a few other architectures did who set
float_tininess_before_rounding.

Will clean this up.

Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding

[Richard Henderson]

On 5/3/23 10:14, Stafford Horne wrote:
>>> +    set_default_nan_mode(1, &cpu->env.fp_status);
>>> +    set_float_detect_tininess(float_tininess_before_rounding,
>>> +                              &cpu->env.fp_status);
>>
>> You don't mention the nan change in the commit message.
> 
> Right, and I am not sure I need it.  Let me remove it and run tests again.  I
> was just adding it as a few other architectures did who set
> float_tininess_before_rounding.

What that does is *not* propagate NaN payloads from (some) input to the output.  This is 
certainly true of RISC-V, which specifies this in their architecture manual.  OpenRISC 
does not specify any NaN behaviour at all.

It's not a bad choice, really, and it almost certainly simplifies the design of the FPU, 
as you can do NaN propagation and silencing in one step.


r~

Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding

[Stafford Horne]

On Wed, May 03, 2023 at 10:41:42AM +0100, Richard Henderson wrote:
> On 5/3/23 10:14, Stafford Horne wrote:
> > > > +    set_default_nan_mode(1, &cpu->env.fp_status);
> > > > +    set_float_detect_tininess(float_tininess_before_rounding,
> > > > +                              &cpu->env.fp_status);
> > > 
> > > You don't mention the nan change in the commit message.
> > 
> > Right, and I am not sure I need it.  Let me remove it and run tests again.  I
> > was just adding it as a few other architectures did who set
> > float_tininess_before_rounding.
> 
> What that does is *not* propagate NaN payloads from (some) input to the
> output.  This is certainly true of RISC-V, which specifies this in their
> architecture manual.  OpenRISC does not specify any NaN behaviour at all.

Thanks, that is what I also gathered from reading up on it.

> It's not a bad choice, really, and it almost certainly simplifies the design
> of the FPU, as you can do NaN propagation and silencing in one step.

Right, it makes sense to optimize.  It doesn't look like any of our FPU
implementation do that at the moment.

I will check with bandvig who implemented the FPU to understand his thought on
this.  It at least deserves to be discussed how nan payload is to be handled in
the architecture spec.

-Stafford