Re: [Qemu-devel] [PATCH v3 3/5] target-mips: remove identical code in different branch

[Aurelien Jarno]

On 2015-06-19 11:08, Leon Alrae wrote:
> Signed-off-by: Leon Alrae <address@hidden>
> ---
>  target-mips/translate.c | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 1d128ee..6fd6dd9 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -11852,11 +11852,7 @@ static int decode_mips16_opc (CPUMIPSState *env, 
> DisasContext *ctx)
>               *      when in debug mode...
>               */
>              check_insn(ctx, ISA_MIPS32);
> -            if (!(ctx->hflags & MIPS_HFLAG_DM)) {
> -                generate_exception(ctx, EXCP_DBp);
> -            } else {
> -                generate_exception(ctx, EXCP_DBp);
> -            }
> +            generate_exception(ctx, EXCP_DBp);

The reason for this duplicated code, is from the comment above, that is
we are not sure which exception should be generated in debug mode. If
someone knows the answer (or my experiment that on real hardware) that 
might be a good opportu to fix that the correct way.

>              break;
>          case RR_SLT:
>              gen_slt(ctx, OPC_SLT, 24, rx, ry);
> @@ -12707,11 +12703,7 @@ static void gen_pool16c_insn(DisasContext *ctx)
>           *      when in debug mode...
>           */
>          check_insn(ctx, ISA_MIPS32);
> -        if (!(ctx->hflags & MIPS_HFLAG_DM)) {
> -            generate_exception(ctx, EXCP_DBp);
> -        } else {
> -            generate_exception(ctx, EXCP_DBp);
> -        }
> +        generate_exception(ctx, EXCP_DBp);
>          break;
>      case JRADDIUSP + 0:
>      case JRADDIUSP + 1:
> @@ -13076,11 +13068,7 @@ static void gen_pool32axf (CPUMIPSState *env, 
> DisasContext *ctx, int rt, int rs)
>              break;
>          case SDBBP:
>              check_insn(ctx, ISA_MIPS32);
> -            if (!(ctx->hflags & MIPS_HFLAG_DM)) {
> -                generate_exception(ctx, EXCP_DBp);
> -            } else {
> -                generate_exception(ctx, EXCP_DBp);
> -            }
> +            generate_exception(ctx, EXCP_DBp);
>              break;
>          default:
>              goto pool32axf_invalid;
> @@ -16849,12 +16837,7 @@ static void decode_opc_special2_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>           *      when in debug mode...
>           */
>          check_insn(ctx, ISA_MIPS32);
> -        if (!(ctx->hflags & MIPS_HFLAG_DM)) {
> -            generate_exception(ctx, EXCP_DBp);
> -        } else {
> -            generate_exception(ctx, EXCP_DBp);
> -        }
> -        /* Treat as NOP. */
> +        generate_exception(ctx, EXCP_DBp);
>          break;
>  #if defined(TARGET_MIPS64)
>      case OPC_DCLO:
> 

Besides the nitpick above:

Reviewed-by: Aurelien Jarno <address@hidden>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net