diff mbox

[v3,3/5] target-mips: remove identical code in different branch

Message ID 1434708524-25434-4-git-send-email-leon.alrae@imgtec.com
State New
Headers show

Commit Message

Leon Alrae June 19, 2015, 10:08 a.m. UTC
Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/translate.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

Comments

Aurelien Jarno June 24, 2015, 2:37 p.m. UTC | #1
On 2015-06-19 11:08, Leon Alrae wrote:
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  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 <aurelien@aurel32.net>
Leon Alrae June 24, 2015, 2:59 p.m. UTC | #2
On 24/06/2015 15:37, Aurelien Jarno wrote:
> On 2015-06-19 11:08, Leon Alrae wrote:
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>> ---
>>  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.

Yes, that's why I left the comment so we are still aware of this problem and
we know where to fix it once we know the answer. But at the time I was
touching this code I felt it should be removed (it generates 4 medium issues
in Coverity).

Leon
diff mbox

Patch

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);
             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: