diff mbox series

[v2,02/15] target/m68k: Switch over exception type in m68k_interrupt_all

Message ID 20211202204900.50973-3-richard.henderson@linaro.org
State Superseded
Headers show
Series target/m68k: Conditional traps + trap cleanup | expand

Commit Message

Richard Henderson Dec. 2, 2021, 8:48 p.m. UTC
Replace an if ladder with a switch for clarity.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/op_helper.c | 45 +++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 3, 2021, 9:02 a.m. UTC | #1
On 12/2/21 21:48, Richard Henderson wrote:
> Replace an if ladder with a switch for clarity.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/m68k/op_helper.c | 45 +++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 17 deletions(-)

> -    } else if (is_hw && oldsr & SR_M &&
> -               cs->exception_index >= EXCP_SPURIOUS &&
> -               cs->exception_index <= EXCP_INT_LEVEL_7) {
> -        do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> -        oldsr = sr;
> -        env->aregs[7] = sp;
> -        cpu_m68k_set_sr(env, sr &= ~SR_M);
> -        sp = env->aregs[7] & ~1;
> -        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> -    } else {
> +        break;
> +
> +    case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
> +        if (is_hw && oldsr & SR_M) {

This is code movement, but adding parenthesis would produce
clearer code.

> +            do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +            oldsr = sr;
> +            env->aregs[7] = sp;
> +            cpu_m68k_set_sr(env, sr &= ~SR_M);

Similarly code movement; this assignation deserves a followup
cleanup patch...

> +            sp = env->aregs[7] & ~1;
> +            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> +            break;
> +        }
> +        /* fall through */
> +
> +    default:
>          do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +        break;
>      }

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Laurent Vivier Dec. 3, 2021, 9:06 a.m. UTC | #2
Le 02/12/2021 à 21:48, Richard Henderson a écrit :
> Replace an if ladder with a switch for clarity.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/op_helper.c | 45 +++++++++++++++++++++++++----------------
>   1 file changed, 28 insertions(+), 17 deletions(-)
> 
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 36662de149..71176eb3d8 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -332,7 +332,8 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>           sp &= ~1;
>       }
>   
> -    if (cs->exception_index == EXCP_ACCESS) {
> +    switch (cs->exception_index) {
> +    case EXCP_ACCESS:
>           if (env->mmu.fault) {
>               cpu_abort(cs, "DOUBLE MMU FAULT\n");
>           }
> @@ -390,26 +391,36 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>                        "ssw:  %08x ea:   %08x sfc:  %d    dfc: %d\n",
>                        env->mmu.ssw, env->mmu.ar, env->sfc, env->dfc);
>           }
> -    } else if (cs->exception_index == EXCP_ADDRESS) {
> +        break;
> +
> +    case EXCP_ADDRESS:
>           do_stack_frame(env, &sp, 2, oldsr, 0, retaddr);
> -    } else if (cs->exception_index == EXCP_ILLEGAL ||
> -               cs->exception_index == EXCP_DIV0 ||
> -               cs->exception_index == EXCP_CHK ||
> -               cs->exception_index == EXCP_TRAPCC ||
> -               cs->exception_index == EXCP_TRACE) {
> +        break;
> +
> +    case EXCP_ILLEGAL:
> +    case EXCP_DIV0:
> +    case EXCP_CHK:
> +    case EXCP_TRAPCC:
> +    case EXCP_TRACE:
>           /* FIXME: addr is not only env->pc */
>           do_stack_frame(env, &sp, 2, oldsr, env->pc, retaddr);
> -    } else if (is_hw && oldsr & SR_M &&
> -               cs->exception_index >= EXCP_SPURIOUS &&
> -               cs->exception_index <= EXCP_INT_LEVEL_7) {
> -        do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> -        oldsr = sr;
> -        env->aregs[7] = sp;
> -        cpu_m68k_set_sr(env, sr &= ~SR_M);
> -        sp = env->aregs[7] & ~1;
> -        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> -    } else {
> +        break;
> +
> +    case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
> +        if (is_hw && oldsr & SR_M) {
> +            do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +            oldsr = sr;
> +            env->aregs[7] = sp;
> +            cpu_m68k_set_sr(env, sr &= ~SR_M);
> +            sp = env->aregs[7] & ~1;
> +            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> +            break;
> +        }
> +        /* fall through */
> +
> +    default:
>           do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +        break;
>       }
>   
>       env->aregs[7] = sp;
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
diff mbox series

Patch

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 36662de149..71176eb3d8 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -332,7 +332,8 @@  static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         sp &= ~1;
     }
 
-    if (cs->exception_index == EXCP_ACCESS) {
+    switch (cs->exception_index) {
+    case EXCP_ACCESS:
         if (env->mmu.fault) {
             cpu_abort(cs, "DOUBLE MMU FAULT\n");
         }
@@ -390,26 +391,36 @@  static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
                      "ssw:  %08x ea:   %08x sfc:  %d    dfc: %d\n",
                      env->mmu.ssw, env->mmu.ar, env->sfc, env->dfc);
         }
-    } else if (cs->exception_index == EXCP_ADDRESS) {
+        break;
+
+    case EXCP_ADDRESS:
         do_stack_frame(env, &sp, 2, oldsr, 0, retaddr);
-    } else if (cs->exception_index == EXCP_ILLEGAL ||
-               cs->exception_index == EXCP_DIV0 ||
-               cs->exception_index == EXCP_CHK ||
-               cs->exception_index == EXCP_TRAPCC ||
-               cs->exception_index == EXCP_TRACE) {
+        break;
+
+    case EXCP_ILLEGAL:
+    case EXCP_DIV0:
+    case EXCP_CHK:
+    case EXCP_TRAPCC:
+    case EXCP_TRACE:
         /* FIXME: addr is not only env->pc */
         do_stack_frame(env, &sp, 2, oldsr, env->pc, retaddr);
-    } else if (is_hw && oldsr & SR_M &&
-               cs->exception_index >= EXCP_SPURIOUS &&
-               cs->exception_index <= EXCP_INT_LEVEL_7) {
-        do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
-        oldsr = sr;
-        env->aregs[7] = sp;
-        cpu_m68k_set_sr(env, sr &= ~SR_M);
-        sp = env->aregs[7] & ~1;
-        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
-    } else {
+        break;
+
+    case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
+        if (is_hw && oldsr & SR_M) {
+            do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+            oldsr = sr;
+            env->aregs[7] = sp;
+            cpu_m68k_set_sr(env, sr &= ~SR_M);
+            sp = env->aregs[7] & ~1;
+            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
+            break;
+        }
+        /* fall through */
+
+    default:
         do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+        break;
     }
 
     env->aregs[7] = sp;