[Qemu-devel] [PATCH v3 04/40] target/mips: Add decode_nanomips_opc() function

Stefan Markovic posted 40 patches 7 years, 6 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 04/40] target/mips: Add decode_nanomips_opc() function
Posted by Stefan Markovic 7 years, 6 months ago
From: Yongbok Kim <yongbok.kim@mips.com>

Add empty body and invocation of decode_nanomips_opc() if the bit
ISA_NANOMIPS32 is set in env->insn_flags.

Signed-off-by: Yongbok Kim <yongbok.kim@mips.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
---
 target/mips/translate.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 227b2c0..67a0f70 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -16458,6 +16458,19 @@ enum {
     NM_EVP      = 0x01,
 };
 
+
+/*
+ *
+ * nanoMIPS decoding engine
+ *
+ */
+
+static int decode_nanomips_opc(CPUMIPSState *env, DisasContext *ctx)
+{
+    return 2;
+}
+
+
 /* SmartMIPS extension to MIPS32 */
 
 #if defined(TARGET_MIPS64)
@@ -21263,8 +21276,13 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
         insn_bytes = 4;
         decode_opc(env, ctx);
     } else if (ctx->insn_flags & ASE_MICROMIPS) {
-        ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
-        insn_bytes = decode_micromips_opc(env, ctx);
+        if (env->insn_flags & ISA_NANOMIPS32) {
+            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
+            insn_bytes = decode_nanomips_opc(env, ctx);
+        } else {
+            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
+            insn_bytes = decode_micromips_opc(env, ctx);
+        }
     } else if (ctx->insn_flags & ASE_MIPS16) {
         ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
         insn_bytes = decode_mips16_opc(env, ctx);
-- 
2.7.4


Re: [Qemu-devel] [PATCH v3 04/40] target/mips: Add decode_nanomips_opc() function
Posted by Richard Henderson 7 years, 6 months ago
On 07/19/2018 05:54 AM, Stefan Markovic wrote:
>          decode_opc(env, ctx);
>      } else if (ctx->insn_flags & ASE_MICROMIPS) {
> -        ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> -        insn_bytes = decode_micromips_opc(env, ctx);
> +        if (env->insn_flags & ISA_NANOMIPS32) {

Be consistent and use ctx->insn_flags.

> +            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> +            insn_bytes = decode_nanomips_opc(env, ctx);
> +        } else {
> +            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> +            insn_bytes = decode_micromips_opc(env, ctx);
> +        }
>      } else if (ctx->insn_flags & ASE_MIPS16) {
>          ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);

Do you really want to nest nanoMIPS within microMIPS?

I would have thought a better structure was

  } else if (ctx->insn_flags & ISA_NANOMIPS32) {
      ...
  } else if (ctx->insn_flags & ASE_MICROMIPS) {
      ...
  } else if (ctx->insn_flags & ASE_MIPS16) {


r~

Re: [Qemu-devel] [PATCH v3 04/40] target/mips: Add decode_nanomips_opc() function
Posted by Aleksandar Markovic 7 years, 6 months ago
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Thursday, July 19, 2018 6:39 PM
> On 07/19/2018 05:54 AM, Stefan Markovic wrote:
> >          decode_opc(env, ctx);
> >      } else if (ctx->insn_flags & ASE_MICROMIPS) {
> > -        ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> > -        insn_bytes = decode_micromips_opc(env, ctx);
> > +        if (env->insn_flags & ISA_NANOMIPS32) {
>
> Be consistent and use ctx->insn_flags.
>
> > +            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> > +            insn_bytes = decode_nanomips_opc(env, ctx);
> > +        } else {
> > +            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> > +            insn_bytes = decode_micromips_opc(env, ctx);
> > +        }
> >      } else if (ctx->insn_flags & ASE_MIPS16) {
> >          ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
>
> Do you really want to nest nanoMIPS within microMIPS?
>
> I would have thought a better structure was
>
>   } else if (ctx->insn_flags & ISA_NANOMIPS32) {
>       ...
>   } else if (ctx->insn_flags & ASE_MICROMIPS) {
>       ...
>   } else if (ctx->insn_flags & ASE_MIPS16) {
>
>
> r~

Hi, Richard,

This will be fixed in the way you described in v4.

Aleksandar