Patchwork [v2] target-mips: fix decoding of microMIPS POOL32Axf instructions

login
register
mail settings
Submitter Leon Alrae
Date Aug. 5, 2013, 4:35 p.m.
Message ID <1375720526-59566-1-git-send-email-leon.alrae@imgtec.com>
Download mbox | patch
Permalink /patch/264708/
State New
Headers show

Comments

Leon Alrae - Aug. 5, 2013, 4:35 p.m.
Fix incorrect assumption that DSP and non-DSP versions of the following
instructions have the same encoding:
MULT, MULTU, MADD, MADDU, MSUB, MSUBU, MFHI, MFLO, MTHI, MTLO.
Correct the existing (non-DSP) instructions and add DSP equivalents.

Reference:
MIPS Architecture for Programmers Volume II-B: The microMIPS32
Instruction Set
MIPS Architecture for Programmers Volume IV-e: The MIPS DSP Module for
the microMIPS32 Architecture

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/translate.c |   58 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 53 insertions(+), 5 deletions(-)
Aurelien Jarno - Aug. 6, 2013, 8:24 a.m.
On Mon, Aug 05, 2013 at 05:35:26PM +0100, Leon Alrae wrote:
> Fix incorrect assumption that DSP and non-DSP versions of the following
> instructions have the same encoding:
> MULT, MULTU, MADD, MADDU, MSUB, MSUBU, MFHI, MFLO, MTHI, MTLO.
> Correct the existing (non-DSP) instructions and add DSP equivalents.
> 
> Reference:
> MIPS Architecture for Programmers Volume II-B: The microMIPS32
> Instruction Set
> MIPS Architecture for Programmers Volume IV-e: The MIPS DSP Module for
> the microMIPS32 Architecture
> 
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  target-mips/translate.c |   58 ++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index c1d57a7..90394a0 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -11061,6 +11061,36 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
>          }
>          break;
>  #endif
> +    case 0x2a:
> +        switch (minor & 3) {
> +        case MADD_ACC:
> +            gen_muldiv(ctx, OPC_MADD, (ctx->opcode >> 14) & 3, rs, rt);
> +            break;
> +        case MADDU_ACC:
> +            gen_muldiv(ctx, OPC_MADDU, (ctx->opcode >> 14) & 3, rs, rt);
> +            break;
> +        case MSUB_ACC:
> +            gen_muldiv(ctx, OPC_MSUB, (ctx->opcode >> 14) & 3, rs, rt);
> +            break;
> +        case MSUBU_ACC:
> +            gen_muldiv(ctx, OPC_MSUBU, (ctx->opcode >> 14) & 3, rs, rt);
> +            break;
> +        default:
> +            goto pool32axf_invalid;
> +        }
> +        break;
> +    case 0x32:
> +        switch (minor & 3) {
> +        case MULT_ACC:
> +            gen_muldiv(ctx, OPC_MULT, (ctx->opcode >> 14) & 3, rs, rt);
> +            break;
> +        case MULTU_ACC:
> +            gen_muldiv(ctx, OPC_MULTU, (ctx->opcode >> 14) & 3, rs, rt);
> +            break;
> +        default:
> +            goto pool32axf_invalid;
> +        }
> +        break;
>      case 0x2c:
>          switch (minor) {
>          case SEB:
> @@ -11113,7 +11143,7 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
>              mips32_op = OPC_MSUBU;
>          do_mul:
>              check_insn(ctx, ISA_MIPS32);
> -            gen_muldiv(ctx, mips32_op, (ctx->opcode >> 14) & 3, rs, rt);
> +            gen_muldiv(ctx, mips32_op, 0, rs, rt);
>              break;
>          default:
>              goto pool32axf_invalid;
> @@ -11247,19 +11277,37 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
>              goto pool32axf_invalid;
>          }
>          break;
> +    case 0x01:
> +        switch (minor & 3) {
> +        case MFHI_ACC:
> +            gen_HILO(ctx, OPC_MFHI, minor >> 2, rs);
> +            break;
> +        case MFLO_ACC:
> +            gen_HILO(ctx, OPC_MFLO, minor >> 2, rs);
> +            break;
> +        case MTHI_ACC:
> +            gen_HILO(ctx, OPC_MTHI, minor >> 2, rs);
> +            break;
> +        case MTLO_ACC:
> +            gen_HILO(ctx, OPC_MTLO, minor >> 2, rs);
> +            break;
> +        default:
> +            goto pool32axf_invalid;
> +        }
> +        break;
>      case 0x35:
>          switch (minor & 3) {

I think this has to be changed into switch (minor) as we don't want to
ignore bits 14 and 15. If they are set, the instruction is invalid.

>          case MFHI32:
> -            gen_HILO(ctx, OPC_MFHI, minor >> 2, rs);
> +            gen_HILO(ctx, OPC_MFHI, 0, rs);
>              break;
>          case MFLO32:
> -            gen_HILO(ctx, OPC_MFLO, minor >> 2, rs);
> +            gen_HILO(ctx, OPC_MFLO, 0, rs);
>              break;
>          case MTHI32:
> -            gen_HILO(ctx, OPC_MTHI, minor >> 2, rs);
> +            gen_HILO(ctx, OPC_MTHI, 0, rs);
>              break;
>          case MTLO32:
> -            gen_HILO(ctx, OPC_MTLO, minor >> 2, rs);
> +            gen_HILO(ctx, OPC_MTLO, 0, rs);
>              break;
>          default:
>              goto pool32axf_invalid;

Beside than the above, the patch looks fine to me. Thanks for the work.

Patch

diff --git a/target-mips/translate.c b/target-mips/translate.c
index c1d57a7..90394a0 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -11061,6 +11061,36 @@  static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
         }
         break;
 #endif
+    case 0x2a:
+        switch (minor & 3) {
+        case MADD_ACC:
+            gen_muldiv(ctx, OPC_MADD, (ctx->opcode >> 14) & 3, rs, rt);
+            break;
+        case MADDU_ACC:
+            gen_muldiv(ctx, OPC_MADDU, (ctx->opcode >> 14) & 3, rs, rt);
+            break;
+        case MSUB_ACC:
+            gen_muldiv(ctx, OPC_MSUB, (ctx->opcode >> 14) & 3, rs, rt);
+            break;
+        case MSUBU_ACC:
+            gen_muldiv(ctx, OPC_MSUBU, (ctx->opcode >> 14) & 3, rs, rt);
+            break;
+        default:
+            goto pool32axf_invalid;
+        }
+        break;
+    case 0x32:
+        switch (minor & 3) {
+        case MULT_ACC:
+            gen_muldiv(ctx, OPC_MULT, (ctx->opcode >> 14) & 3, rs, rt);
+            break;
+        case MULTU_ACC:
+            gen_muldiv(ctx, OPC_MULTU, (ctx->opcode >> 14) & 3, rs, rt);
+            break;
+        default:
+            goto pool32axf_invalid;
+        }
+        break;
     case 0x2c:
         switch (minor) {
         case SEB:
@@ -11113,7 +11143,7 @@  static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
             mips32_op = OPC_MSUBU;
         do_mul:
             check_insn(ctx, ISA_MIPS32);
-            gen_muldiv(ctx, mips32_op, (ctx->opcode >> 14) & 3, rs, rt);
+            gen_muldiv(ctx, mips32_op, 0, rs, rt);
             break;
         default:
             goto pool32axf_invalid;
@@ -11247,19 +11277,37 @@  static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
             goto pool32axf_invalid;
         }
         break;
+    case 0x01:
+        switch (minor & 3) {
+        case MFHI_ACC:
+            gen_HILO(ctx, OPC_MFHI, minor >> 2, rs);
+            break;
+        case MFLO_ACC:
+            gen_HILO(ctx, OPC_MFLO, minor >> 2, rs);
+            break;
+        case MTHI_ACC:
+            gen_HILO(ctx, OPC_MTHI, minor >> 2, rs);
+            break;
+        case MTLO_ACC:
+            gen_HILO(ctx, OPC_MTLO, minor >> 2, rs);
+            break;
+        default:
+            goto pool32axf_invalid;
+        }
+        break;
     case 0x35:
         switch (minor & 3) {
         case MFHI32:
-            gen_HILO(ctx, OPC_MFHI, minor >> 2, rs);
+            gen_HILO(ctx, OPC_MFHI, 0, rs);
             break;
         case MFLO32:
-            gen_HILO(ctx, OPC_MFLO, minor >> 2, rs);
+            gen_HILO(ctx, OPC_MFLO, 0, rs);
             break;
         case MTHI32:
-            gen_HILO(ctx, OPC_MTHI, minor >> 2, rs);
+            gen_HILO(ctx, OPC_MTHI, 0, rs);
             break;
         case MTLO32:
-            gen_HILO(ctx, OPC_MTLO, minor >> 2, rs);
+            gen_HILO(ctx, OPC_MTLO, 0, rs);
             break;
         default:
             goto pool32axf_invalid;