* [PATCH 0/3] OpenRISC updates for user space FPU
@ 2023-05-02 18:57 Stafford Horne
2023-05-02 18:57 ` [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode Stafford Horne
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Stafford Horne @ 2023-05-02 18:57 UTC (permalink / raw)
To: QEMU Development; +Cc: Linux OpenRISC, 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
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode 2023-05-02 18:57 [PATCH 0/3] OpenRISC updates for user space FPU Stafford Horne @ 2023-05-02 18:57 ` Stafford Horne 2023-05-03 6:29 ` Stafford Horne 2023-05-02 18:57 ` [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception Stafford Horne 2023-05-02 18:57 ` [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding Stafford Horne 2 siblings, 1 reply; 11+ messages in thread From: Stafford Horne @ 2023-05-02 18:57 UTC (permalink / raw) To: QEMU Development; +Cc: Linux OpenRISC, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode 2023-05-02 18:57 ` [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode Stafford Horne @ 2023-05-03 6:29 ` Stafford Horne 0 siblings, 0 replies; 11+ messages in thread From: Stafford Horne @ 2023-05-03 6:29 UTC (permalink / raw) To: QEMU Development; +Cc: Linux OpenRISC 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception 2023-05-02 18:57 [PATCH 0/3] OpenRISC updates for user space FPU Stafford Horne 2023-05-02 18:57 ` [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode Stafford Horne @ 2023-05-02 18:57 ` Stafford Horne 2023-05-03 7:36 ` Richard Henderson 2023-05-02 18:57 ` [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding Stafford Horne 2 siblings, 1 reply; 11+ messages in thread From: Stafford Horne @ 2023-05-02 18:57 UTC (permalink / raw) To: QEMU Development; +Cc: Linux OpenRISC, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception 2023-05-02 18:57 ` [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception Stafford Horne @ 2023-05-03 7:36 ` Richard Henderson 2023-05-03 9:12 ` Stafford Horne 0 siblings, 1 reply; 11+ messages in thread From: Richard Henderson @ 2023-05-03 7:36 UTC (permalink / raw) To: Stafford Horne, QEMU Development; +Cc: Linux OpenRISC 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~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception 2023-05-03 7:36 ` Richard Henderson @ 2023-05-03 9:12 ` Stafford Horne 0 siblings, 0 replies; 11+ messages in thread From: Stafford Horne @ 2023-05-03 9:12 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Development, Linux OpenRISC 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding 2023-05-02 18:57 [PATCH 0/3] OpenRISC updates for user space FPU Stafford Horne 2023-05-02 18:57 ` [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode Stafford Horne 2023-05-02 18:57 ` [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception Stafford Horne @ 2023-05-02 18:57 ` Stafford Horne 2023-05-03 7:37 ` Richard Henderson 2 siblings, 1 reply; 11+ messages in thread From: Stafford Horne @ 2023-05-02 18:57 UTC (permalink / raw) To: QEMU Development; +Cc: Linux OpenRISC, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding 2023-05-02 18:57 ` [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding Stafford Horne @ 2023-05-03 7:37 ` Richard Henderson 2023-05-03 9:14 ` Stafford Horne 0 siblings, 1 reply; 11+ messages in thread From: Richard Henderson @ 2023-05-03 7:37 UTC (permalink / raw) To: Stafford Horne, QEMU Development; +Cc: Linux OpenRISC 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~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding 2023-05-03 7:37 ` Richard Henderson @ 2023-05-03 9:14 ` Stafford Horne 2023-05-03 9:41 ` Richard Henderson 0 siblings, 1 reply; 11+ messages in thread From: Stafford Horne @ 2023-05-03 9:14 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Development, Linux OpenRISC 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding 2023-05-03 9:14 ` Stafford Horne @ 2023-05-03 9:41 ` Richard Henderson 2023-05-03 16:31 ` Stafford Horne 0 siblings, 1 reply; 11+ messages in thread From: Richard Henderson @ 2023-05-03 9:41 UTC (permalink / raw) To: Stafford Horne; +Cc: QEMU Development, Linux OpenRISC 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~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding 2023-05-03 9:41 ` Richard Henderson @ 2023-05-03 16:31 ` Stafford Horne 0 siblings, 0 replies; 11+ messages in thread From: Stafford Horne @ 2023-05-03 16:31 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Development, Linux OpenRISC 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-03 16:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-02 18:57 [PATCH 0/3] OpenRISC updates for user space FPU Stafford Horne 2023-05-02 18:57 ` [PATCH 1/3] target/openrisc: Allow fpcsr access in user mode Stafford Horne 2023-05-03 6:29 ` Stafford Horne 2023-05-02 18:57 ` [PATCH 2/3] target/openrisc: Set PC to cpu state on FPU exception Stafford Horne 2023-05-03 7:36 ` Richard Henderson 2023-05-03 9:12 ` Stafford Horne 2023-05-02 18:57 ` [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding Stafford Horne 2023-05-03 7:37 ` Richard Henderson 2023-05-03 9:14 ` Stafford Horne 2023-05-03 9:41 ` Richard Henderson 2023-05-03 16:31 ` Stafford Horne
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).