diff mbox

target-or32: Improve float exception

Message ID 1353550204-6510-1-git-send-email-gf91597@gmail.com
State New
Headers show

Commit Message

Feng Gao Nov. 22, 2012, 2:10 a.m. UTC 
From: gaofeng <gaofeng@gaofeng-K43SM.(none)>

When float exception raise, can save right pc.

Signed-off-by: Feng Gao <gf91597@gmail.com>
---
 target-openrisc/exception.c  |   35 ++++++++++++++++++++++++++++--
 target-openrisc/exception.h  |    8 ++++++-
 target-openrisc/fpu_helper.c |   48 +++++++++++++++++++++++-------------------
 target-openrisc/mmu_helper.c |   17 +++------------
 4 files changed, 69 insertions(+), 39 deletions(-)

Comments

Peter Maydell Nov. 22, 2012, 5:27 p.m. UTC | #1 
On 22 November 2012 02:10, Feng Gao <gf91597@gmail.com> wrote:
> From: gaofeng <gaofeng@gaofeng-K43SM.(none)>

This doesn't look like a valid From line...

> When float exception raise, can save right pc.
>
> Signed-off-by: Feng Gao <gf91597@gmail.com>
> ---
>  target-openrisc/exception.c  |   35 ++++++++++++++++++++++++++++--
>  target-openrisc/exception.h  |    8 ++++++-
>  target-openrisc/fpu_helper.c |   48 +++++++++++++++++++++++-------------------
>  target-openrisc/mmu_helper.c |   17 +++------------
>  4 files changed, 69 insertions(+), 39 deletions(-)
>
> diff --git a/target-openrisc/exception.c b/target-openrisc/exception.c
> index 58e53c6..f37e32c 100644
> --- a/target-openrisc/exception.c
> +++ b/target-openrisc/exception.c
> @@ -20,8 +20,39 @@
>  #include "cpu.h"
>  #include "exception.h"
>
> -void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t excp)
> +void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t exception)
>  {
> -    cpu->env.exception_index = excp;
> +    do_raise_exception_err(cpu, exception, 0);
> +}
> +
> +void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
> +                                          uint32_t exception,
> +                                          uintptr_t pc)
> +{
> +    TranslationBlock *tb;
> +#if 1
> +    if (exception < 0x100)
> +        qemu_log("%s: %d\n", __func__, exception);
> +#endif

Stray debug tracing?

> +    cpu->env.exception_index = exception;
> +
> +    if (pc) {
> +        /* now we have a real cpu fault */
> +        tb = tb_find_pc(pc);
> +        if (tb) {
> +            /* the PC is inside the translated code. It means that we have
> +               a virtual CPU fault */
> +            cpu_restore_state(tb, &cpu->env, pc);
> +        }
> +    }
> +
>      cpu_loop_exit(&cpu->env);
>  }
> +
> +void QEMU_NORETURN do_raise_exception(OpenRISCCPU *cpu,
> +                                      uint32_t exception,
> +                                      uintptr_t pc)
> +{
> +    do_raise_exception_err(cpu, exception, pc);
> +}

What's this wrapper for?

-- PMM
陳韋任 Nov. 22, 2012, 6:42 p.m. UTC | #2 
> -void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t excp)
> +void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t exception)
>  {
> -    cpu->env.exception_index = excp;
> +    do_raise_exception_err(cpu, exception, 0);
> +}
> +
> +void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
> +                                          uint32_t exception,
> +                                          uintptr_t pc)
> +{
> +    TranslationBlock *tb;
> +#if 1
> +    if (exception < 0x100)
> +        qemu_log("%s: %d\n", __func__, exception);
> +#endif
> +    cpu->env.exception_index = exception;
> +
> +    if (pc) {
> +        /* now we have a real cpu fault */
> +        tb = tb_find_pc(pc);
> +        if (tb) {
> +            /* the PC is inside the translated code. It means that we have
> +               a virtual CPU fault */
> +            cpu_restore_state(tb, &cpu->env, pc);
> +        }
> +    }
> +
>      cpu_loop_exit(&cpu->env);
>  }
> +
> +void QEMU_NORETURN do_raise_exception(OpenRISCCPU *cpu,
> +                                      uint32_t exception,
> +                                      uintptr_t pc)
> +{
> +    do_raise_exception_err(cpu, exception, pc);
> +}

  I don't see any difference between do_raise_exception_err and
do_raise_exception here. MIPS do have those functions, but function
do_raise_exception_err has one more parameter for error code.
Please see target-mips/op_helper.c. If you don't need error code,
there just one function would be sufficient, I think.

> diff --git a/target-openrisc/exception.h b/target-openrisc/exception.h
> index 4b64430..1f0cd90 100644
> --- a/target-openrisc/exception.h
> +++ b/target-openrisc/exception.h
> @@ -23,6 +23,12 @@
>  #include "cpu.h"
>  #include "qemu-common.h"
>  
> -void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t excp);
> +void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t exception);
>  
> +void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
> +                                          uint32_t exception,
> +                                          uintptr_t pc);
> +void QEMU_NORETURN do_raise_exception(OpenRISCCPU *cpu,
> +                                      uint32_t exception,
> +                                      uintptr_t pc);

  Ditto.

Regards,
chenwj
陳韋任 Nov. 22, 2012, 6:45 p.m. UTC | #3 
> > +void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
> > +                                          uint32_t exception,
> > +                                          uintptr_t pc)
> > +{
> > +    TranslationBlock *tb;
> > +#if 1
> > +    if (exception < 0x100)
> > +        qemu_log("%s: %d\n", __func__, exception);
> > +#endif
> 
> Stray debug tracing?

  target-mips/op_helper.c also has such "#if 1 .. #endif".
Should we remove it?

Regards,
chenwj
Peter Maydell Nov. 22, 2012, 7:06 p.m. UTC | #4 
On 22 November 2012 18:45, 陳韋任 (Wei-Ren Chen) <chenwj@iis.sinica.edu.tw> wrote:
>> > +void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
>> > +                                          uint32_t exception,
>> > +                                          uintptr_t pc)
>> > +{
>> > +    TranslationBlock *tb;
>> > +#if 1
>> > +    if (exception < 0x100)
>> > +        qemu_log("%s: %d\n", __func__, exception);
>> > +#endif
>>
>> Stray debug tracing?
>
>   target-mips/op_helper.c also has such "#if 1 .. #endif".
> Should we remove it?

Well, qemu_log() only actually does anything if logging
is enabled [which I had forgotten], so that part is OK.
However I don't think we should have the #if 1..#endif.
Either we want the code or we don't; #if 1 or #if 0 is
rarely a good idea.

Also, (1) why are exceptions < 0x100 the only interesting
ones? (2) does this really apply equally to mips and openrisc,
or was it just an accidental code copying? (3) braces :-).

-- PMM
diff mbox

Patch

diff --git a/target-openrisc/exception.c b/target-openrisc/exception.c
index 58e53c6..f37e32c 100644
--- a/target-openrisc/exception.c
+++ b/target-openrisc/exception.c
@@ -20,8 +20,39 @@ 
 #include "cpu.h"
 #include "exception.h"
 
-void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t excp)
+void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t exception)
 {
-    cpu->env.exception_index = excp;
+    do_raise_exception_err(cpu, exception, 0);
+}
+
+void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
+                                          uint32_t exception,
+                                          uintptr_t pc)
+{
+    TranslationBlock *tb;
+#if 1
+    if (exception < 0x100)
+        qemu_log("%s: %d\n", __func__, exception);
+#endif
+    cpu->env.exception_index = exception;
+
+    if (pc) {
+        /* now we have a real cpu fault */
+        tb = tb_find_pc(pc);
+        if (tb) {
+            /* the PC is inside the translated code. It means that we have
+               a virtual CPU fault */
+            cpu_restore_state(tb, &cpu->env, pc);
+        }
+    }
+
     cpu_loop_exit(&cpu->env);
 }
+
+void QEMU_NORETURN do_raise_exception(OpenRISCCPU *cpu,
+                                      uint32_t exception,
+                                      uintptr_t pc)
+{
+    do_raise_exception_err(cpu, exception, pc);
+}
+
diff --git a/target-openrisc/exception.h b/target-openrisc/exception.h
index 4b64430..1f0cd90 100644
--- a/target-openrisc/exception.h
+++ b/target-openrisc/exception.h
@@ -23,6 +23,12 @@ 
 #include "cpu.h"
 #include "qemu-common.h"
 
-void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t excp);
+void QEMU_NORETURN raise_exception(OpenRISCCPU *cpu, uint32_t exception);
 
+void QEMU_NORETURN do_raise_exception_err(OpenRISCCPU *cpu,
+                                          uint32_t exception,
+                                          uintptr_t pc);
+void QEMU_NORETURN do_raise_exception(OpenRISCCPU *cpu,
+                                      uint32_t exception,
+                                      uintptr_t pc);
 #endif /* QEMU_OPENRISC_EXCP_H */
diff --git a/target-openrisc/fpu_helper.c b/target-openrisc/fpu_helper.c
index b184d5e..c2890d5 100644
--- a/target-openrisc/fpu_helper.c
+++ b/target-openrisc/fpu_helper.c
@@ -51,17 +51,21 @@  static inline uint32_t ieee_ex_to_openrisc(OpenRISCCPU *cpu, int fexcp)
     return ret;
 }
 
-static inline void update_fpcsr(OpenRISCCPU *cpu)
+static inline void update_fpcsr(OpenRISCCPU *cpu, uintptr_t pc)
 {
     int tmp = ieee_ex_to_openrisc(cpu,
                               get_float_exception_flags(&cpu->env.fp_status));
 
     SET_FP_CAUSE(cpu->env.fpcsr, tmp);
-    if ((GET_FP_ENABLE(cpu->env.fpcsr) & tmp) &&
-        (cpu->env.fpcsr & FPCSR_FPEE)) {
-        helper_exception(&cpu->env, EXCP_FPE);
-    } else {
-        UPDATE_FP_FLAGS(cpu->env.fpcsr, tmp);
+    if (tmp) {
+        set_float_exception_flags(0, &cpu->env.fp_status);
+
+        if ((GET_FP_ENABLE(cpu->env.fpcsr) & tmp) &&
+            (cpu->env.fpcsr & FPCSR_FPEE)) {
+            do_raise_exception(cpu, EXCP_FPE, pc);
+        } else {
+            UPDATE_FP_FLAGS(cpu->env.fpcsr, tmp);
+        }
     }
 }
 
@@ -72,7 +76,7 @@  uint64_t HELPER(itofd)(CPUOpenRISCState *env, uint64_t val)
 
     set_float_exception_flags(0, &cpu->env.fp_status);
     itofd = int32_to_float64(val, &cpu->env.fp_status);
-    update_fpcsr(cpu);
+    update_fpcsr(cpu, GETPC());
 
     return itofd;
 }
@@ -84,7 +88,7 @@  uint32_t HELPER(itofs)(CPUOpenRISCState *env, uint32_t val)
 
     set_float_exception_flags(0, &cpu->env.fp_status);
     itofs = int32_to_float32(val, &cpu->env.fp_status);
-    update_fpcsr(cpu);
+    update_fpcsr(cpu, GETPC());
 
     return itofs;
 }
@@ -96,7 +100,7 @@  uint64_t HELPER(ftoid)(CPUOpenRISCState *env, uint64_t val)
 
     set_float_exception_flags(0, &cpu->env.fp_status);
     ftoid = float32_to_int64(val, &cpu->env.fp_status);
-    update_fpcsr(cpu);
+    update_fpcsr(cpu, GETPC());
 
     return ftoid;
 }
@@ -108,7 +112,7 @@  uint32_t HELPER(ftois)(CPUOpenRISCState *env, uint32_t val)
 
     set_float_exception_flags(0, &cpu->env.fp_status);
     ftois = float32_to_int32(val, &cpu->env.fp_status);
-    update_fpcsr(cpu);
+    update_fpcsr(cpu, GETPC());
 
     return ftois;
 }
@@ -123,7 +127,7 @@  uint64_t helper_float_ ## name ## _d(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     result = float64_ ## name(fdt0, fdt1, &cpu->env.fp_status);           \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return result;                                                        \
 }                                                                         \
                                                                           \
@@ -134,7 +138,7 @@  uint32_t helper_float_ ## name ## _s(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     result = float32_ ## name(fdt0, fdt1, &cpu->env.fp_status);           \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return result;                                                        \
 }                                                                         \
 
@@ -163,7 +167,7 @@  uint64_t helper_float_ ## name1 ## name2 ## _d(CPUOpenRISCState *env,     \
     result = float64_ ## name2(result, temp, &cpu->env.fp_status);        \
     val1 = result >> 32;                                                  \
     val2 = (uint32_t) (result & 0xffffffff);                              \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     cpu->env.fpmaddlo = val2;                                             \
     cpu->env.fpmaddhi = val1;                                             \
     return 0;                                                             \
@@ -183,7 +187,7 @@  uint32_t helper_float_ ## name1 ## name2 ## _s(CPUOpenRISCState *env,     \
     result = float64_ ## name2(result, temp, &cpu->env.fp_status);        \
     val1 = result >> 32;                                                  \
     val2 = (uint32_t) (result & 0xffffffff);                              \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     cpu->env.fpmaddlo = val2;                                             \
     cpu->env.fpmaddhi = val1;                                             \
     return 0;                                                             \
@@ -201,7 +205,7 @@  uint64_t helper_float_ ## name ## _d(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = float64_ ## name(fdt0, fdt1, &cpu->env.fp_status);              \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }                                                                         \
                                                                           \
@@ -212,7 +216,7 @@  uint32_t helper_float_ ## name ## _s(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = float32_ ## name(fdt0, fdt1, &cpu->env.fp_status);              \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }
 
@@ -230,7 +234,7 @@  uint64_t helper_float_ ## name ## _d(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = !float64_eq_quiet(fdt0, fdt1, &cpu->env.fp_status);             \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }                                                                         \
                                                                           \
@@ -241,7 +245,7 @@  uint32_t helper_float_ ## name ## _s(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = !float32_eq_quiet(fdt0, fdt1, &cpu->env.fp_status);             \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }
 
@@ -256,7 +260,7 @@  uint64_t helper_float_ ## name ## _d(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = !float64_le(fdt0, fdt1, &cpu->env.fp_status);                   \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }                                                                         \
                                                                           \
@@ -267,7 +271,7 @@  uint32_t helper_float_ ## name ## _s(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = !float32_le(fdt0, fdt1, &cpu->env.fp_status);                   \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }
 FLOAT_CMPGT(gt)
@@ -281,7 +285,7 @@  uint64_t helper_float_ ## name ## _d(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = !float64_lt(fdt0, fdt1, &cpu->env.fp_status);                   \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }                                                                         \
                                                                           \
@@ -292,7 +296,7 @@  uint32_t helper_float_ ## name ## _s(CPUOpenRISCState *env,               \
     OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));                    \
     set_float_exception_flags(0, &cpu->env.fp_status);                    \
     res = !float32_lt(fdt0, fdt1, &cpu->env.fp_status);                   \
-    update_fpcsr(cpu);                                                    \
+    update_fpcsr(cpu, GETPC());                                           \
     return res;                                                           \
 }
 
diff --git a/target-openrisc/mmu_helper.c b/target-openrisc/mmu_helper.c
index 59ed371..8b687c0 100644
--- a/target-openrisc/mmu_helper.c
+++ b/target-openrisc/mmu_helper.c
@@ -19,6 +19,7 @@ 
  */
 
 #include "cpu.h"
+#include "exception.h"
 
 #ifndef CONFIG_USER_ONLY
 #include "softmmu_exec.h"
@@ -39,25 +40,13 @@ 
 void tlb_fill(CPUOpenRISCState *env, target_ulong addr, int is_write,
               int mmu_idx, uintptr_t retaddr)
 {
-    TranslationBlock *tb;
-    unsigned long pc;
     int ret;
 
+    OpenRISCCPU *cpu = OPENRISC_CPU(ENV_GET_CPU(env));
     ret = cpu_openrisc_handle_mmu_fault(env, addr, is_write, mmu_idx);
 
     if (ret) {
-        if (retaddr) {
-            /* now we have a real cpu fault.  */
-            pc = (unsigned long)retaddr;
-            tb = tb_find_pc(pc);
-            if (tb) {
-                /* the PC is inside the translated code. It means that we
-                   have a virtual CPU fault.  */
-                cpu_restore_state(tb, env, pc);
-            }
-        }
-        /* Raise Exception.  */
-        cpu_loop_exit(env);
+        do_raise_exception_err(cpu, env->exception_index, retaddr); 
     }
 }
 #endif