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

[Laurent Vivier]

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>