[PATCH 0/9] target/mips: Simplify MSA TCG logic

[PATCH 0/9] target/mips: Simplify MSA TCG logic

[Philippe Mathieu-Daudé]

I converted MSA opcodes to decodetree. To keep the series
small I split it in 2, this is the non-decodetree specific
patches (so non-decodetree experts can review it ;) ).

First we stop using env->insn_flags to check for MSAi
presence, then we restrict TCG functions to DisasContext*.

Based-on: <20201130102228.2395100-1-f4bug@amsat.org>
"target/mips: Allow executing MSA instructions on Loongson-3A4000"

Philippe Mathieu-Daudé (9):
  target/mips: Introduce ase_msa_available() helper
  target/mips: Simplify msa_reset()
  target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA
  target/mips: Simplify MSA TCG logic
  target/mips: Remove now unused ASE_MSA definition
  target/mips: Alias MSA vector registers on FPU scalar registers
  target/mips: Extract msa_translate_init() from mips_tcg_init()
  target/mips: Remove CPUMIPSState* argument from gen_msa*() methods
  target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

 target/mips/internal.h           |   8 +-
 target/mips/mips-defs.h          |   1 -
 target/mips/kvm.c                |  12 +-
 target/mips/translate.c          | 206 ++++++++++++++++++-------------
 target/mips/translate_init.c.inc |  12 +-
 5 files changed, 138 insertions(+), 101 deletions(-)

-- 
2.26.2

[PATCH 0/9] target/mips: Simplify MSA TCG logic

[Philippe Mathieu-Daudé]

I converted MSA opcodes to decodetree. To keep the series
small I split it in 2, this is the non-decodetree specific
patches (so non-decodetree experts can review it ;) ).

First we stop using env->insn_flags to check for MSAi
presence, then we restrict TCG functions to DisasContext*.

Based-on: <20201130102228.2395100-1-f4bug@amsat.org>
"target/mips: Allow executing MSA instructions on Loongson-3A4000"

Philippe Mathieu-Daudé (9):
  target/mips: Introduce ase_msa_available() helper
  target/mips: Simplify msa_reset()
  target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA
  target/mips: Simplify MSA TCG logic
  target/mips: Remove now unused ASE_MSA definition
  target/mips: Alias MSA vector registers on FPU scalar registers
  target/mips: Extract msa_translate_init() from mips_tcg_init()
  target/mips: Remove CPUMIPSState* argument from gen_msa*() methods
  target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

 target/mips/internal.h           |   8 +-
 target/mips/mips-defs.h          |   1 -
 target/mips/kvm.c                |  12 +-
 target/mips/translate.c          | 206 ++++++++++++++++++-------------
 target/mips/translate_init.c.inc |  12 +-
 5 files changed, 138 insertions(+), 101 deletions(-)

-- 
2.26.2

[PATCH 1/9] target/mips: Introduce ase_msa_available() helper

[Philippe Mathieu-Daudé]

Instead of accessing CP0_Config3 directly and checking
the 'MSA Present' bit, introduce an explicit helper,
making the code easier to read.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/internal.h  |  6 ++++++
 target/mips/kvm.c       | 12 ++++++------
 target/mips/translate.c |  8 +++-----
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index dd8a7809b64..f882ac1580c 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -80,6 +80,12 @@ enum CPUMIPSMSADataFormat {
     DF_DOUBLE
 };
 
+/* Check presence of MSA implementation */
+static inline bool ase_msa_available(CPUMIPSState *env)
+{
+    return env->CP0_Config3 & (1 << CP0C3_MSAP);
+}
+
 void mips_cpu_do_interrupt(CPUState *cpu);
 bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
 void mips_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 72637a1e021..9bfd67ede39 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -81,7 +81,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
-    if (kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (kvm_mips_msa_cap && ase_msa_available(env)) {
         ret = kvm_vcpu_enable_cap(cs, KVM_CAP_MIPS_MSA, 0, 0);
         if (ret < 0) {
             /* mark unsupported so it gets disabled on reset */
@@ -107,7 +107,7 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
         warn_report("KVM does not support FPU, disabling");
         env->CP0_Config1 &= ~(1 << CP0C1_FP);
     }
-    if (!kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (!kvm_mips_msa_cap && ase_msa_available(env)) {
         warn_report("KVM does not support MSA, disabling");
         env->CP0_Config3 &= ~(1 << CP0C3_MSAP);
     }
@@ -624,7 +624,7 @@ static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
          * FPU register state is a subset of MSA vector state, so don't put FPU
          * registers if we're emulating a CPU with MSA.
          */
-        if (!(env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if (!ase_msa_available(env)) {
             /* Floating point registers */
             for (i = 0; i < 32; ++i) {
                 if (env->CP0_Status & (1 << CP0St_FR)) {
@@ -643,7 +643,7 @@ static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
     }
 
     /* Only put MSA state if we're emulating a CPU with MSA */
-    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (ase_msa_available(env)) {
         /* MSA Control Registers */
         if (level == KVM_PUT_FULL_STATE) {
             err = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_MSA_IR,
@@ -704,7 +704,7 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
          * FPU register state is a subset of MSA vector state, so don't save FPU
          * registers if we're emulating a CPU with MSA.
          */
-        if (!(env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if (!ase_msa_available(env)) {
             /* Floating point registers */
             for (i = 0; i < 32; ++i) {
                 if (env->CP0_Status & (1 << CP0St_FR)) {
@@ -723,7 +723,7 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
     }
 
     /* Only get MSA state if we're emulating a CPU with MSA */
-    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (ase_msa_available(env)) {
         /* MSA Control Registers */
         err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_MSA_IR,
                                    &env->msair);
diff --git a/target/mips/translate.c b/target/mips/translate.c
index c64a1bc42e1..a7c01c2ea5b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -25049,8 +25049,7 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
         gen_trap(ctx, op1, rs, rt, -1);
         break;
     case OPC_LSA: /* OPC_PMON */
-        if ((ctx->insn_flags & ISA_MIPS32R6) ||
-            (env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if ((ctx->insn_flags & ISA_MIPS32R6) || ase_msa_available(env)) {
             decode_opc_special_r6(env, ctx);
         } else {
             /* Pmon entry point, also R4010 selsl */
@@ -25152,8 +25151,7 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case OPC_DLSA:
-        if ((ctx->insn_flags & ISA_MIPS32R6) ||
-            (env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if ((ctx->insn_flags & ISA_MIPS32R6) || ase_msa_available(env)) {
             decode_opc_special_r6(env, ctx);
         }
         break;
@@ -32000,7 +31998,7 @@ void cpu_state_reset(CPUMIPSState *env)
     }
 
     /* MSA */
-    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (ase_msa_available(env)) {
         msa_reset(env);
     }
 
-- 
2.26.2

[PATCH 1/9] target/mips: Introduce ase_msa_available() helper

[Philippe Mathieu-Daudé]

Instead of accessing CP0_Config3 directly and checking
the 'MSA Present' bit, introduce an explicit helper,
making the code easier to read.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/internal.h  |  6 ++++++
 target/mips/kvm.c       | 12 ++++++------
 target/mips/translate.c |  8 +++-----
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index dd8a7809b64..f882ac1580c 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -80,6 +80,12 @@ enum CPUMIPSMSADataFormat {
     DF_DOUBLE
 };
 
+/* Check presence of MSA implementation */
+static inline bool ase_msa_available(CPUMIPSState *env)
+{
+    return env->CP0_Config3 & (1 << CP0C3_MSAP);
+}
+
 void mips_cpu_do_interrupt(CPUState *cpu);
 bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
 void mips_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 72637a1e021..9bfd67ede39 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -81,7 +81,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
-    if (kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (kvm_mips_msa_cap && ase_msa_available(env)) {
         ret = kvm_vcpu_enable_cap(cs, KVM_CAP_MIPS_MSA, 0, 0);
         if (ret < 0) {
             /* mark unsupported so it gets disabled on reset */
@@ -107,7 +107,7 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
         warn_report("KVM does not support FPU, disabling");
         env->CP0_Config1 &= ~(1 << CP0C1_FP);
     }
-    if (!kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (!kvm_mips_msa_cap && ase_msa_available(env)) {
         warn_report("KVM does not support MSA, disabling");
         env->CP0_Config3 &= ~(1 << CP0C3_MSAP);
     }
@@ -624,7 +624,7 @@ static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
          * FPU register state is a subset of MSA vector state, so don't put FPU
          * registers if we're emulating a CPU with MSA.
          */
-        if (!(env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if (!ase_msa_available(env)) {
             /* Floating point registers */
             for (i = 0; i < 32; ++i) {
                 if (env->CP0_Status & (1 << CP0St_FR)) {
@@ -643,7 +643,7 @@ static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
     }
 
     /* Only put MSA state if we're emulating a CPU with MSA */
-    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (ase_msa_available(env)) {
         /* MSA Control Registers */
         if (level == KVM_PUT_FULL_STATE) {
             err = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_MSA_IR,
@@ -704,7 +704,7 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
          * FPU register state is a subset of MSA vector state, so don't save FPU
          * registers if we're emulating a CPU with MSA.
          */
-        if (!(env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if (!ase_msa_available(env)) {
             /* Floating point registers */
             for (i = 0; i < 32; ++i) {
                 if (env->CP0_Status & (1 << CP0St_FR)) {
@@ -723,7 +723,7 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
     }
 
     /* Only get MSA state if we're emulating a CPU with MSA */
-    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (ase_msa_available(env)) {
         /* MSA Control Registers */
         err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_MSA_IR,
                                    &env->msair);
diff --git a/target/mips/translate.c b/target/mips/translate.c
index c64a1bc42e1..a7c01c2ea5b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -25049,8 +25049,7 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
         gen_trap(ctx, op1, rs, rt, -1);
         break;
     case OPC_LSA: /* OPC_PMON */
-        if ((ctx->insn_flags & ISA_MIPS32R6) ||
-            (env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if ((ctx->insn_flags & ISA_MIPS32R6) || ase_msa_available(env)) {
             decode_opc_special_r6(env, ctx);
         } else {
             /* Pmon entry point, also R4010 selsl */
@@ -25152,8 +25151,7 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case OPC_DLSA:
-        if ((ctx->insn_flags & ISA_MIPS32R6) ||
-            (env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+        if ((ctx->insn_flags & ISA_MIPS32R6) || ase_msa_available(env)) {
             decode_opc_special_r6(env, ctx);
         }
         break;
@@ -32000,7 +31998,7 @@ void cpu_state_reset(CPUMIPSState *env)
     }
 
     /* MSA */
-    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+    if (ase_msa_available(env)) {
         msa_reset(env);
     }
 
-- 
2.26.2

[PATCH 2/9] target/mips: Simplify msa_reset()

[Philippe Mathieu-Daudé]

Call msa_reset() inconditionally, but only reset
the MSA registers if MSA is implemented.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Maybe not very useful.
---
 target/mips/translate.c          | 5 +----
 target/mips/translate_init.c.inc | 4 ++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a7c01c2ea5b..803ffefba2c 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -31997,10 +31997,7 @@ void cpu_state_reset(CPUMIPSState *env)
         env->hflags |= MIPS_HFLAG_M16;
     }
 
-    /* MSA */
-    if (ase_msa_available(env)) {
-        msa_reset(env);
-    }
+    msa_reset(env);
 
     compute_hflags(env);
     restore_fp_status(env);
diff --git a/target/mips/translate_init.c.inc b/target/mips/translate_init.c.inc
index 79f75ed863c..3b069190ed8 100644
--- a/target/mips/translate_init.c.inc
+++ b/target/mips/translate_init.c.inc
@@ -1018,6 +1018,10 @@ static void mvp_init (CPUMIPSState *env, const mips_def_t *def)
 
 static void msa_reset(CPUMIPSState *env)
 {
+    if (!ase_msa_available(env)) {
+        return;
+    }
+
 #ifdef CONFIG_USER_ONLY
     /* MSA access enabled */
     env->CP0_Config5 |= 1 << CP0C5_MSAEn;
-- 
2.26.2

[PATCH 2/9] target/mips: Simplify msa_reset()

[Philippe Mathieu-Daudé]

Call msa_reset() inconditionally, but only reset
the MSA registers if MSA is implemented.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Maybe not very useful.
---
 target/mips/translate.c          | 5 +----
 target/mips/translate_init.c.inc | 4 ++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a7c01c2ea5b..803ffefba2c 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -31997,10 +31997,7 @@ void cpu_state_reset(CPUMIPSState *env)
         env->hflags |= MIPS_HFLAG_M16;
     }
 
-    /* MSA */
-    if (ase_msa_available(env)) {
-        msa_reset(env);
-    }
+    msa_reset(env);
 
     compute_hflags(env);
     restore_fp_status(env);
diff --git a/target/mips/translate_init.c.inc b/target/mips/translate_init.c.inc
index 79f75ed863c..3b069190ed8 100644
--- a/target/mips/translate_init.c.inc
+++ b/target/mips/translate_init.c.inc
@@ -1018,6 +1018,10 @@ static void mvp_init (CPUMIPSState *env, const mips_def_t *def)
 
 static void msa_reset(CPUMIPSState *env)
 {
+    if (!ase_msa_available(env)) {
+        return;
+    }
+
 #ifdef CONFIG_USER_ONLY
     /* MSA access enabled */
     env->CP0_Config5 |= 1 << CP0C5_MSAEn;
-- 
2.26.2

[PATCH 3/9] target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA

[Philippe Mathieu-Daudé]

MSA presence is expressed by the MSAP bit of CP0_Config3.
We don't need to check anything else.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index f882ac1580c..95cbd314018 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -433,7 +433,7 @@ static inline void compute_hflags(CPUMIPSState *env)
             env->hflags |= MIPS_HFLAG_COP1X;
         }
     }
-    if (env->insn_flags & ASE_MSA) {
+    if (ase_msa_available(env)) {
         if (env->CP0_Config5 & (1 << CP0C5_MSAEn)) {
             env->hflags |= MIPS_HFLAG_MSA;
         }
-- 
2.26.2

[PATCH 3/9] target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA

[Philippe Mathieu-Daudé]

MSA presence is expressed by the MSAP bit of CP0_Config3.
We don't need to check anything else.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index f882ac1580c..95cbd314018 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -433,7 +433,7 @@ static inline void compute_hflags(CPUMIPSState *env)
             env->hflags |= MIPS_HFLAG_COP1X;
         }
     }
-    if (env->insn_flags & ASE_MSA) {
+    if (ase_msa_available(env)) {
         if (env->CP0_Config5 & (1 << CP0C5_MSAEn)) {
             env->hflags |= MIPS_HFLAG_MSA;
         }
-- 
2.26.2

[PATCH 4/9] target/mips: Simplify MSA TCG logic

[Philippe Mathieu-Daudé]

Only decode MSA opcodes if MSA is present (implemented).

Now than check_msa_access() will only be called if MSA is
present, the only way to have MIPS_HFLAG_MSA unset is if
MSA is disabled (bit CP0C5_MSAEn cleared, see previous
commit). Therefore we can remove the 'reserved instruction'
exception.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 803ffefba2c..a05c25e50b8 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28697,13 +28697,8 @@ static inline int check_msa_access(DisasContext *ctx)
     }
 
     if (unlikely(!(ctx->hflags & MIPS_HFLAG_MSA))) {
-        if (ctx->insn_flags & ASE_MSA) {
-            generate_exception_end(ctx, EXCP_MSADIS);
-            return 0;
-        } else {
-            generate_exception_end(ctx, EXCP_RI);
-            return 0;
-        }
+        generate_exception_end(ctx, EXCP_MSADIS);
+        return 0;
     }
     return 1;
 }
@@ -30547,7 +30542,7 @@ static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
 static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opcode = ctx->opcode;
-    check_insn(ctx, ASE_MSA);
+
     check_msa_access(ctx);
 
     switch (MASK_MSA_MINOR(opcode)) {
@@ -31194,9 +31189,10 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         case OPC_BNZ_H:
         case OPC_BNZ_W:
         case OPC_BNZ_D:
-            check_insn(ctx, ASE_MSA);
-            gen_msa_branch(env, ctx, op1);
-            break;
+            if (ase_msa_available(env)) {
+                gen_msa_branch(env, ctx, op1);
+                break;
+            }
         default:
             MIPS_INVAL("cp1");
             generate_exception_end(ctx, EXCP_RI);
@@ -31385,7 +31381,9 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
 #endif
         } else {
             /* MDMX: Not implemented. */
-            gen_msa(env, ctx);
+            if (ase_msa_available(env)) {
+                gen_msa(env, ctx);
+            }
         }
         break;
     case OPC_PCREL:
-- 
2.26.2

[PATCH 4/9] target/mips: Simplify MSA TCG logic

[Philippe Mathieu-Daudé]

Only decode MSA opcodes if MSA is present (implemented).

Now than check_msa_access() will only be called if MSA is
present, the only way to have MIPS_HFLAG_MSA unset is if
MSA is disabled (bit CP0C5_MSAEn cleared, see previous
commit). Therefore we can remove the 'reserved instruction'
exception.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 803ffefba2c..a05c25e50b8 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28697,13 +28697,8 @@ static inline int check_msa_access(DisasContext *ctx)
     }
 
     if (unlikely(!(ctx->hflags & MIPS_HFLAG_MSA))) {
-        if (ctx->insn_flags & ASE_MSA) {
-            generate_exception_end(ctx, EXCP_MSADIS);
-            return 0;
-        } else {
-            generate_exception_end(ctx, EXCP_RI);
-            return 0;
-        }
+        generate_exception_end(ctx, EXCP_MSADIS);
+        return 0;
     }
     return 1;
 }
@@ -30547,7 +30542,7 @@ static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
 static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opcode = ctx->opcode;
-    check_insn(ctx, ASE_MSA);
+
     check_msa_access(ctx);
 
     switch (MASK_MSA_MINOR(opcode)) {
@@ -31194,9 +31189,10 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         case OPC_BNZ_H:
         case OPC_BNZ_W:
         case OPC_BNZ_D:
-            check_insn(ctx, ASE_MSA);
-            gen_msa_branch(env, ctx, op1);
-            break;
+            if (ase_msa_available(env)) {
+                gen_msa_branch(env, ctx, op1);
+                break;
+            }
         default:
             MIPS_INVAL("cp1");
             generate_exception_end(ctx, EXCP_RI);
@@ -31385,7 +31381,9 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
 #endif
         } else {
             /* MDMX: Not implemented. */
-            gen_msa(env, ctx);
+            if (ase_msa_available(env)) {
+                gen_msa(env, ctx);
+            }
         }
         break;
     case OPC_PCREL:
-- 
2.26.2

[PATCH 5/9] target/mips: Remove now unused ASE_MSA definition

[Philippe Mathieu-Daudé]

We don't use ASE_MSA anymore (replaced by ase_msa_available()
checking MSAP bit from CP0_Config3). Remove it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/mips-defs.h          | 1 -
 target/mips/translate_init.c.inc | 8 ++++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index ed6a7a9e545..805034b8956 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -45,7 +45,6 @@
 #define ASE_MT            0x0000000040000000ULL
 #define ASE_SMARTMIPS     0x0000000080000000ULL
 #define ASE_MICROMIPS     0x0000000100000000ULL
-#define ASE_MSA           0x0000000200000000ULL
 /*
  *   bits 40-51: vendor-specific base instruction sets
  */
diff --git a/target/mips/translate_init.c.inc b/target/mips/translate_init.c.inc
index 3b069190ed8..2170f8ace6f 100644
--- a/target/mips/translate_init.c.inc
+++ b/target/mips/translate_init.c.inc
@@ -408,7 +408,7 @@ const mips_def_t mips_defs[] =
         .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
         .SEGBITS = 32,
         .PABITS = 40,
-        .insn_flags = CPU_MIPS32R5 | ASE_MSA,
+        .insn_flags = CPU_MIPS32R5,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
@@ -719,7 +719,7 @@ const mips_def_t mips_defs[] =
         .MSAIR = 0x03 << MSAIR_ProcID,
         .SEGBITS = 48,
         .PABITS = 48,
-        .insn_flags = CPU_MIPS64R6 | ASE_MSA,
+        .insn_flags = CPU_MIPS64R6,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
@@ -759,7 +759,7 @@ const mips_def_t mips_defs[] =
         .MSAIR = 0x03 << MSAIR_ProcID,
         .SEGBITS = 48,
         .PABITS = 48,
-        .insn_flags = CPU_MIPS64R6 | ASE_MSA,
+        .insn_flags = CPU_MIPS64R6,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
@@ -885,7 +885,7 @@ const mips_def_t mips_defs[] =
         .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
         .SEGBITS = 48,
         .PABITS = 48,
-        .insn_flags = CPU_LOONGSON3A | ASE_MSA,
+        .insn_flags = CPU_LOONGSON3A,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
-- 
2.26.2

[PATCH 5/9] target/mips: Remove now unused ASE_MSA definition

[Philippe Mathieu-Daudé]

We don't use ASE_MSA anymore (replaced by ase_msa_available()
checking MSAP bit from CP0_Config3). Remove it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/mips-defs.h          | 1 -
 target/mips/translate_init.c.inc | 8 ++++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index ed6a7a9e545..805034b8956 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -45,7 +45,6 @@
 #define ASE_MT            0x0000000040000000ULL
 #define ASE_SMARTMIPS     0x0000000080000000ULL
 #define ASE_MICROMIPS     0x0000000100000000ULL
-#define ASE_MSA           0x0000000200000000ULL
 /*
  *   bits 40-51: vendor-specific base instruction sets
  */
diff --git a/target/mips/translate_init.c.inc b/target/mips/translate_init.c.inc
index 3b069190ed8..2170f8ace6f 100644
--- a/target/mips/translate_init.c.inc
+++ b/target/mips/translate_init.c.inc
@@ -408,7 +408,7 @@ const mips_def_t mips_defs[] =
         .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
         .SEGBITS = 32,
         .PABITS = 40,
-        .insn_flags = CPU_MIPS32R5 | ASE_MSA,
+        .insn_flags = CPU_MIPS32R5,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
@@ -719,7 +719,7 @@ const mips_def_t mips_defs[] =
         .MSAIR = 0x03 << MSAIR_ProcID,
         .SEGBITS = 48,
         .PABITS = 48,
-        .insn_flags = CPU_MIPS64R6 | ASE_MSA,
+        .insn_flags = CPU_MIPS64R6,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
@@ -759,7 +759,7 @@ const mips_def_t mips_defs[] =
         .MSAIR = 0x03 << MSAIR_ProcID,
         .SEGBITS = 48,
         .PABITS = 48,
-        .insn_flags = CPU_MIPS64R6 | ASE_MSA,
+        .insn_flags = CPU_MIPS64R6,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
@@ -885,7 +885,7 @@ const mips_def_t mips_defs[] =
         .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
         .SEGBITS = 48,
         .PABITS = 48,
-        .insn_flags = CPU_LOONGSON3A | ASE_MSA,
+        .insn_flags = CPU_LOONGSON3A,
         .mmu_type = MMU_TYPE_R4000,
     },
     {
-- 
2.26.2

[PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Philippe Mathieu-Daudé]

Commits 863f264d10f ("add msa_reset(), global msa register") and
cb269f273fd ("fix multiple TCG registers covering same data")
removed the FPU scalar registers and replaced them by aliases to
the MSA vector registers.
While this might be the case for CPU implementing MSA, this makes
QEMU code incoherent for CPU not implementing it. It is simpler
to inverse the logic and alias the MSA vector registers on the
FPU scalar ones.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a05c25e50b8..41880f21abd 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -31682,16 +31682,20 @@ void mips_tcg_init(void)
                                         offsetof(CPUMIPSState,
                                                  active_tc.gpr[i]),
                                         regnames[i]);
-
     for (i = 0; i < 32; i++) {
         int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
-        msa_wr_d[i * 2] =
-                tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
+
+        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
+    }
+    /* MSA */
+    for (i = 0; i < 32; i++) {
+        int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
+
         /*
-         * The scalar floating-point unit (FPU) registers are mapped on
-         * the MSA vector registers.
+         * The MSA vector registers are mapped on the
+         * scalar floating-point unit (FPU) registers.
          */
-        fpu_f64[i] = msa_wr_d[i * 2];
+        msa_wr_d[i * 2] = fpu_f64[i];
         off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[1]);
         msa_wr_d[i * 2 + 1] =
                 tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2 + 1]);
-- 
2.26.2

[PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Philippe Mathieu-Daudé]

Commits 863f264d10f ("add msa_reset(), global msa register") and
cb269f273fd ("fix multiple TCG registers covering same data")
removed the FPU scalar registers and replaced them by aliases to
the MSA vector registers.
While this might be the case for CPU implementing MSA, this makes
QEMU code incoherent for CPU not implementing it. It is simpler
to inverse the logic and alias the MSA vector registers on the
FPU scalar ones.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a05c25e50b8..41880f21abd 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -31682,16 +31682,20 @@ void mips_tcg_init(void)
                                         offsetof(CPUMIPSState,
                                                  active_tc.gpr[i]),
                                         regnames[i]);
-
     for (i = 0; i < 32; i++) {
         int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
-        msa_wr_d[i * 2] =
-                tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
+
+        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
+    }
+    /* MSA */
+    for (i = 0; i < 32; i++) {
+        int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
+
         /*
-         * The scalar floating-point unit (FPU) registers are mapped on
-         * the MSA vector registers.
+         * The MSA vector registers are mapped on the
+         * scalar floating-point unit (FPU) registers.
          */
-        fpu_f64[i] = msa_wr_d[i * 2];
+        msa_wr_d[i * 2] = fpu_f64[i];
         off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[1]);
         msa_wr_d[i * 2 + 1] =
                 tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2 + 1]);
-- 
2.26.2

[PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Philippe Mathieu-Daudé]

Extract the logic initialization of the MSA registers from
the generic initialization.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 41880f21abd..a5112acc351 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -31672,6 +31672,24 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 }
 
+static void msa_translate_init(void)
+{
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
+
+        /*
+         * The MSA vector registers are mapped on the
+         * scalar floating-point unit (FPU) registers.
+         */
+        msa_wr_d[i * 2] = fpu_f64[i];
+        off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[1]);
+        msa_wr_d[i * 2 + 1] =
+                tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2 + 1]);
+    }
+}
+
 void mips_tcg_init(void)
 {
     int i;
@@ -31685,22 +31703,9 @@ void mips_tcg_init(void)
     for (i = 0; i < 32; i++) {
         int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
 
-        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
+        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
     }
-    /* MSA */
-    for (i = 0; i < 32; i++) {
-        int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
-
-        /*
-         * The MSA vector registers are mapped on the
-         * scalar floating-point unit (FPU) registers.
-         */
-        msa_wr_d[i * 2] = fpu_f64[i];
-        off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[1]);
-        msa_wr_d[i * 2 + 1] =
-                tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2 + 1]);
-    }
-
+    msa_translate_init();
     cpu_PC = tcg_global_mem_new(cpu_env,
                                 offsetof(CPUMIPSState, active_tc.PC), "PC");
     for (i = 0; i < MIPS_DSP_ACC; i++) {
-- 
2.26.2

[PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Philippe Mathieu-Daudé]

Extract the logic initialization of the MSA registers from
the generic initialization.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 41880f21abd..a5112acc351 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -31672,6 +31672,24 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 }
 
+static void msa_translate_init(void)
+{
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
+
+        /*
+         * The MSA vector registers are mapped on the
+         * scalar floating-point unit (FPU) registers.
+         */
+        msa_wr_d[i * 2] = fpu_f64[i];
+        off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[1]);
+        msa_wr_d[i * 2 + 1] =
+                tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2 + 1]);
+    }
+}
+
 void mips_tcg_init(void)
 {
     int i;
@@ -31685,22 +31703,9 @@ void mips_tcg_init(void)
     for (i = 0; i < 32; i++) {
         int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
 
-        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
+        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
     }
-    /* MSA */
-    for (i = 0; i < 32; i++) {
-        int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
-
-        /*
-         * The MSA vector registers are mapped on the
-         * scalar floating-point unit (FPU) registers.
-         */
-        msa_wr_d[i * 2] = fpu_f64[i];
-        off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[1]);
-        msa_wr_d[i * 2 + 1] =
-                tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2 + 1]);
-    }
-
+    msa_translate_init();
     cpu_PC = tcg_global_mem_new(cpu_env,
                                 offsetof(CPUMIPSState, active_tc.PC), "PC");
     for (i = 0; i < MIPS_DSP_ACC; i++) {
-- 
2.26.2

[PATCH 8/9] target/mips: Remove CPUMIPSState* argument from gen_msa*() methods

[Philippe Mathieu-Daudé]

The gen_msa*() methods don't use the "CPUMIPSState *env"
argument. Remove it to simplify.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 57 ++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a5112acc351..5311e6ced62 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28744,7 +28744,7 @@ static void gen_check_zero_element(TCGv tresult, uint8_t df, uint8_t wt)
     tcg_temp_free_i64(t1);
 }
 
-static void gen_msa_branch(CPUMIPSState *env, DisasContext *ctx, uint32_t op1)
+static void gen_msa_branch(DisasContext *ctx, uint32_t op1)
 {
     uint8_t df = (ctx->opcode >> 21) & 0x3;
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
@@ -28789,7 +28789,7 @@ static void gen_msa_branch(CPUMIPSState *env, DisasContext *ctx, uint32_t op1)
     ctx->hflags |= MIPS_HFLAG_BDS32;
 }
 
-static void gen_msa_i8(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_i8(DisasContext *ctx)
 {
 #define MASK_MSA_I8(op)    (MASK_MSA_MINOR(op) | (op & (0x03 << 24)))
     uint8_t i8 = (ctx->opcode >> 16) & 0xff;
@@ -28847,7 +28847,7 @@ static void gen_msa_i8(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(ti8);
 }
 
-static void gen_msa_i5(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_i5(DisasContext *ctx)
 {
 #define MASK_MSA_I5(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
     uint8_t df = (ctx->opcode >> 21) & 0x3;
@@ -28920,7 +28920,7 @@ static void gen_msa_i5(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(timm);
 }
 
-static void gen_msa_bit(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_bit(DisasContext *ctx)
 {
 #define MASK_MSA_BIT(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
     uint8_t dfm = (ctx->opcode >> 16) & 0x7f;
@@ -29004,7 +29004,7 @@ static void gen_msa_bit(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tws);
 }
 
-static void gen_msa_3r(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_3r(DisasContext *ctx)
 {
 #define MASK_MSA_3R(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
     uint8_t df = (ctx->opcode >> 21) & 0x3;
@@ -29986,7 +29986,7 @@ static void gen_msa_3r(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_elm_3e(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_elm_3e(DisasContext *ctx)
 {
 #define MASK_MSA_ELM_DF3E(op)   (MASK_MSA_MINOR(op) | (op & (0x3FF << 16)))
     uint8_t source = (ctx->opcode >> 11) & 0x1f;
@@ -30018,8 +30018,7 @@ static void gen_msa_elm_3e(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tsr);
 }
 
-static void gen_msa_elm_df(CPUMIPSState *env, DisasContext *ctx, uint32_t df,
-        uint32_t n)
+static void gen_msa_elm_df(DisasContext *ctx, uint32_t df, uint32_t n)
 {
 #define MASK_MSA_ELM(op)    (MASK_MSA_MINOR(op) | (op & (0xf << 22)))
     uint8_t ws = (ctx->opcode >> 11) & 0x1f;
@@ -30129,7 +30128,7 @@ static void gen_msa_elm_df(CPUMIPSState *env, DisasContext *ctx, uint32_t df,
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_elm(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_elm(DisasContext *ctx)
 {
     uint8_t dfn = (ctx->opcode >> 16) & 0x3f;
     uint32_t df = 0, n = 0;
@@ -30148,17 +30147,17 @@ static void gen_msa_elm(CPUMIPSState *env, DisasContext *ctx)
         df = DF_DOUBLE;
     } else if (dfn == 0x3E) {
         /* CTCMSA, CFCMSA, MOVE.V */
-        gen_msa_elm_3e(env, ctx);
+        gen_msa_elm_3e(ctx);
         return;
     } else {
         generate_exception_end(ctx, EXCP_RI);
         return;
     }
 
-    gen_msa_elm_df(env, ctx, df, n);
+    gen_msa_elm_df(ctx, df, n);
 }
 
-static void gen_msa_3rf(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_3rf(DisasContext *ctx)
 {
 #define MASK_MSA_3RF(op)    (MASK_MSA_MINOR(op) | (op & (0xf << 22)))
     uint8_t df = (ctx->opcode >> 21) & 0x1;
@@ -30316,7 +30315,7 @@ static void gen_msa_3rf(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_2r(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_2r(DisasContext *ctx)
 {
 #define MASK_MSA_2R(op)     (MASK_MSA_MINOR(op) | (op & (0x1f << 21)) | \
                             (op & (0x7 << 18)))
@@ -30400,7 +30399,7 @@ static void gen_msa_2r(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_2rf(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_2rf(DisasContext *ctx)
 {
 #define MASK_MSA_2RF(op)    (MASK_MSA_MINOR(op) | (op & (0x1f << 21)) | \
                             (op & (0xf << 17)))
@@ -30471,7 +30470,7 @@ static void gen_msa_2rf(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_vec_v(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_vec_v(DisasContext *ctx)
 {
 #define MASK_MSA_VEC(op)    (MASK_MSA_MINOR(op) | (op & (0x1f << 21)))
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
@@ -30514,7 +30513,7 @@ static void gen_msa_vec_v(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(twt);
 }
 
-static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_vec(DisasContext *ctx)
 {
     switch (MASK_MSA_VEC(ctx->opcode)) {
     case OPC_AND_V:
@@ -30524,13 +30523,13 @@ static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
     case OPC_BMNZ_V:
     case OPC_BMZ_V:
     case OPC_BSEL_V:
-        gen_msa_vec_v(env, ctx);
+        gen_msa_vec_v(ctx);
         break;
     case OPC_MSA_2R:
-        gen_msa_2r(env, ctx);
+        gen_msa_2r(ctx);
         break;
     case OPC_MSA_2RF:
-        gen_msa_2rf(env, ctx);
+        gen_msa_2rf(ctx);
         break;
     default:
         MIPS_INVAL("MSA instruction");
@@ -30539,7 +30538,7 @@ static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
-static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa(DisasContext *ctx)
 {
     uint32_t opcode = ctx->opcode;
 
@@ -30549,15 +30548,15 @@ static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MSA_I8_00:
     case OPC_MSA_I8_01:
     case OPC_MSA_I8_02:
-        gen_msa_i8(env, ctx);
+        gen_msa_i8(ctx);
         break;
     case OPC_MSA_I5_06:
     case OPC_MSA_I5_07:
-        gen_msa_i5(env, ctx);
+        gen_msa_i5(ctx);
         break;
     case OPC_MSA_BIT_09:
     case OPC_MSA_BIT_0A:
-        gen_msa_bit(env, ctx);
+        gen_msa_bit(ctx);
         break;
     case OPC_MSA_3R_0D:
     case OPC_MSA_3R_0E:
@@ -30568,18 +30567,18 @@ static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MSA_3R_13:
     case OPC_MSA_3R_14:
     case OPC_MSA_3R_15:
-        gen_msa_3r(env, ctx);
+        gen_msa_3r(ctx);
         break;
     case OPC_MSA_ELM:
-        gen_msa_elm(env, ctx);
+        gen_msa_elm(ctx);
         break;
     case OPC_MSA_3RF_1A:
     case OPC_MSA_3RF_1B:
     case OPC_MSA_3RF_1C:
-        gen_msa_3rf(env, ctx);
+        gen_msa_3rf(ctx);
         break;
     case OPC_MSA_VEC:
-        gen_msa_vec(env, ctx);
+        gen_msa_vec(ctx);
         break;
     case OPC_LD_B:
     case OPC_LD_H:
@@ -31190,7 +31189,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         case OPC_BNZ_W:
         case OPC_BNZ_D:
             if (ase_msa_available(env)) {
-                gen_msa_branch(env, ctx, op1);
+                gen_msa_branch(ctx, op1);
                 break;
             }
         default:
@@ -31382,7 +31381,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         } else {
             /* MDMX: Not implemented. */
             if (ase_msa_available(env)) {
-                gen_msa(env, ctx);
+                gen_msa(ctx);
             }
         }
         break;
-- 
2.26.2

[PATCH 8/9] target/mips: Remove CPUMIPSState* argument from gen_msa*() methods

[Philippe Mathieu-Daudé]

The gen_msa*() methods don't use the "CPUMIPSState *env"
argument. Remove it to simplify.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 57 ++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a5112acc351..5311e6ced62 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28744,7 +28744,7 @@ static void gen_check_zero_element(TCGv tresult, uint8_t df, uint8_t wt)
     tcg_temp_free_i64(t1);
 }
 
-static void gen_msa_branch(CPUMIPSState *env, DisasContext *ctx, uint32_t op1)
+static void gen_msa_branch(DisasContext *ctx, uint32_t op1)
 {
     uint8_t df = (ctx->opcode >> 21) & 0x3;
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
@@ -28789,7 +28789,7 @@ static void gen_msa_branch(CPUMIPSState *env, DisasContext *ctx, uint32_t op1)
     ctx->hflags |= MIPS_HFLAG_BDS32;
 }
 
-static void gen_msa_i8(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_i8(DisasContext *ctx)
 {
 #define MASK_MSA_I8(op)    (MASK_MSA_MINOR(op) | (op & (0x03 << 24)))
     uint8_t i8 = (ctx->opcode >> 16) & 0xff;
@@ -28847,7 +28847,7 @@ static void gen_msa_i8(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(ti8);
 }
 
-static void gen_msa_i5(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_i5(DisasContext *ctx)
 {
 #define MASK_MSA_I5(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
     uint8_t df = (ctx->opcode >> 21) & 0x3;
@@ -28920,7 +28920,7 @@ static void gen_msa_i5(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(timm);
 }
 
-static void gen_msa_bit(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_bit(DisasContext *ctx)
 {
 #define MASK_MSA_BIT(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
     uint8_t dfm = (ctx->opcode >> 16) & 0x7f;
@@ -29004,7 +29004,7 @@ static void gen_msa_bit(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tws);
 }
 
-static void gen_msa_3r(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_3r(DisasContext *ctx)
 {
 #define MASK_MSA_3R(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
     uint8_t df = (ctx->opcode >> 21) & 0x3;
@@ -29986,7 +29986,7 @@ static void gen_msa_3r(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_elm_3e(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_elm_3e(DisasContext *ctx)
 {
 #define MASK_MSA_ELM_DF3E(op)   (MASK_MSA_MINOR(op) | (op & (0x3FF << 16)))
     uint8_t source = (ctx->opcode >> 11) & 0x1f;
@@ -30018,8 +30018,7 @@ static void gen_msa_elm_3e(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tsr);
 }
 
-static void gen_msa_elm_df(CPUMIPSState *env, DisasContext *ctx, uint32_t df,
-        uint32_t n)
+static void gen_msa_elm_df(DisasContext *ctx, uint32_t df, uint32_t n)
 {
 #define MASK_MSA_ELM(op)    (MASK_MSA_MINOR(op) | (op & (0xf << 22)))
     uint8_t ws = (ctx->opcode >> 11) & 0x1f;
@@ -30129,7 +30128,7 @@ static void gen_msa_elm_df(CPUMIPSState *env, DisasContext *ctx, uint32_t df,
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_elm(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_elm(DisasContext *ctx)
 {
     uint8_t dfn = (ctx->opcode >> 16) & 0x3f;
     uint32_t df = 0, n = 0;
@@ -30148,17 +30147,17 @@ static void gen_msa_elm(CPUMIPSState *env, DisasContext *ctx)
         df = DF_DOUBLE;
     } else if (dfn == 0x3E) {
         /* CTCMSA, CFCMSA, MOVE.V */
-        gen_msa_elm_3e(env, ctx);
+        gen_msa_elm_3e(ctx);
         return;
     } else {
         generate_exception_end(ctx, EXCP_RI);
         return;
     }
 
-    gen_msa_elm_df(env, ctx, df, n);
+    gen_msa_elm_df(ctx, df, n);
 }
 
-static void gen_msa_3rf(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_3rf(DisasContext *ctx)
 {
 #define MASK_MSA_3RF(op)    (MASK_MSA_MINOR(op) | (op & (0xf << 22)))
     uint8_t df = (ctx->opcode >> 21) & 0x1;
@@ -30316,7 +30315,7 @@ static void gen_msa_3rf(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_2r(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_2r(DisasContext *ctx)
 {
 #define MASK_MSA_2R(op)     (MASK_MSA_MINOR(op) | (op & (0x1f << 21)) | \
                             (op & (0x7 << 18)))
@@ -30400,7 +30399,7 @@ static void gen_msa_2r(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_2rf(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_2rf(DisasContext *ctx)
 {
 #define MASK_MSA_2RF(op)    (MASK_MSA_MINOR(op) | (op & (0x1f << 21)) | \
                             (op & (0xf << 17)))
@@ -30471,7 +30470,7 @@ static void gen_msa_2rf(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(tdf);
 }
 
-static void gen_msa_vec_v(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_vec_v(DisasContext *ctx)
 {
 #define MASK_MSA_VEC(op)    (MASK_MSA_MINOR(op) | (op & (0x1f << 21)))
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
@@ -30514,7 +30513,7 @@ static void gen_msa_vec_v(CPUMIPSState *env, DisasContext *ctx)
     tcg_temp_free_i32(twt);
 }
 
-static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa_vec(DisasContext *ctx)
 {
     switch (MASK_MSA_VEC(ctx->opcode)) {
     case OPC_AND_V:
@@ -30524,13 +30523,13 @@ static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
     case OPC_BMNZ_V:
     case OPC_BMZ_V:
     case OPC_BSEL_V:
-        gen_msa_vec_v(env, ctx);
+        gen_msa_vec_v(ctx);
         break;
     case OPC_MSA_2R:
-        gen_msa_2r(env, ctx);
+        gen_msa_2r(ctx);
         break;
     case OPC_MSA_2RF:
-        gen_msa_2rf(env, ctx);
+        gen_msa_2rf(ctx);
         break;
     default:
         MIPS_INVAL("MSA instruction");
@@ -30539,7 +30538,7 @@ static void gen_msa_vec(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
-static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
+static void gen_msa(DisasContext *ctx)
 {
     uint32_t opcode = ctx->opcode;
 
@@ -30549,15 +30548,15 @@ static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MSA_I8_00:
     case OPC_MSA_I8_01:
     case OPC_MSA_I8_02:
-        gen_msa_i8(env, ctx);
+        gen_msa_i8(ctx);
         break;
     case OPC_MSA_I5_06:
     case OPC_MSA_I5_07:
-        gen_msa_i5(env, ctx);
+        gen_msa_i5(ctx);
         break;
     case OPC_MSA_BIT_09:
     case OPC_MSA_BIT_0A:
-        gen_msa_bit(env, ctx);
+        gen_msa_bit(ctx);
         break;
     case OPC_MSA_3R_0D:
     case OPC_MSA_3R_0E:
@@ -30568,18 +30567,18 @@ static void gen_msa(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MSA_3R_13:
     case OPC_MSA_3R_14:
     case OPC_MSA_3R_15:
-        gen_msa_3r(env, ctx);
+        gen_msa_3r(ctx);
         break;
     case OPC_MSA_ELM:
-        gen_msa_elm(env, ctx);
+        gen_msa_elm(ctx);
         break;
     case OPC_MSA_3RF_1A:
     case OPC_MSA_3RF_1B:
     case OPC_MSA_3RF_1C:
-        gen_msa_3rf(env, ctx);
+        gen_msa_3rf(ctx);
         break;
     case OPC_MSA_VEC:
-        gen_msa_vec(env, ctx);
+        gen_msa_vec(ctx);
         break;
     case OPC_LD_B:
     case OPC_LD_H:
@@ -31190,7 +31189,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         case OPC_BNZ_W:
         case OPC_BNZ_D:
             if (ase_msa_available(env)) {
-                gen_msa_branch(env, ctx, op1);
+                gen_msa_branch(ctx, op1);
                 break;
             }
         default:
@@ -31382,7 +31381,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         } else {
             /* MDMX: Not implemented. */
             if (ase_msa_available(env)) {
-                gen_msa(env, ctx);
+                gen_msa(ctx);
             }
         }
         break;
-- 
2.26.2

[PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Philippe Mathieu-Daudé]

In preparation of using the decodetree script, explode
gen_msa_branch() as following:

- OPC_BZ_V              -> BxZ_V(EQ)
- OPC_BNZ_V             -> BxZ_V(NE)
- OPC_BZ_[BHWD]         -> BxZ(false)
- OPC_BNZ_[BHWD]        -> BxZ(true)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 71 ++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 22 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 5311e6ced62..8a35d4d0d03 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28744,49 +28744,76 @@ static void gen_check_zero_element(TCGv tresult, uint8_t df, uint8_t wt)
     tcg_temp_free_i64(t1);
 }
 
+static bool gen_msa_BxZ_V(DisasContext *ctx, int wt, int s16, TCGCond cond)
+{
+    TCGv_i64 t0;
+
+    check_msa_access(ctx);
+
+    if (ctx->hflags & MIPS_HFLAG_BMASK) {
+        generate_exception_end(ctx, EXCP_RI);
+        return true;
+    }
+    t0 = tcg_temp_new_i64();
+    tcg_gen_or_i64(t0, msa_wr_d[wt << 1], msa_wr_d[(wt << 1) + 1]);
+    tcg_gen_setcondi_i64(cond, t0, t0, 0);
+    tcg_gen_trunc_i64_tl(bcond, t0);
+    tcg_temp_free_i64(t0);
+
+    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
+
+    ctx->hflags |= MIPS_HFLAG_BC;
+    ctx->hflags |= MIPS_HFLAG_BDS32;
+
+    return true;
+}
+
+static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
+{
+    check_msa_access(ctx);
+
+    if (ctx->hflags & MIPS_HFLAG_BMASK) {
+        generate_exception_end(ctx, EXCP_RI);
+        return true;
+    }
+
+    gen_check_zero_element(bcond, df, wt);
+    if (if_not) {
+        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
+    }
+
+    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
+    ctx->hflags |= MIPS_HFLAG_BC;
+    ctx->hflags |= MIPS_HFLAG_BDS32;
+
+    return true;
+}
+
 static void gen_msa_branch(DisasContext *ctx, uint32_t op1)
 {
     uint8_t df = (ctx->opcode >> 21) & 0x3;
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
     int64_t s16 = (int16_t)ctx->opcode;
 
-    check_msa_access(ctx);
-
-    if (ctx->hflags & MIPS_HFLAG_BMASK) {
-        generate_exception_end(ctx, EXCP_RI);
-        return;
-    }
     switch (op1) {
     case OPC_BZ_V:
     case OPC_BNZ_V:
-        {
-            TCGv_i64 t0 = tcg_temp_new_i64();
-            tcg_gen_or_i64(t0, msa_wr_d[wt << 1], msa_wr_d[(wt << 1) + 1]);
-            tcg_gen_setcondi_i64((op1 == OPC_BZ_V) ?
-                    TCG_COND_EQ : TCG_COND_NE, t0, t0, 0);
-            tcg_gen_trunc_i64_tl(bcond, t0);
-            tcg_temp_free_i64(t0);
-        }
+        gen_msa_BxZ_V(ctx, wt, s16, (op1 == OPC_BZ_V) ?
+                                    TCG_COND_EQ : TCG_COND_NE);
         break;
     case OPC_BZ_B:
     case OPC_BZ_H:
     case OPC_BZ_W:
     case OPC_BZ_D:
-        gen_check_zero_element(bcond, df, wt);
+        gen_msa_BxZ(ctx, df, wt, s16, false);
         break;
     case OPC_BNZ_B:
     case OPC_BNZ_H:
     case OPC_BNZ_W:
     case OPC_BNZ_D:
-        gen_check_zero_element(bcond, df, wt);
-        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
+        gen_msa_BxZ(ctx, df, wt, s16, true);
         break;
     }
-
-    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
-
-    ctx->hflags |= MIPS_HFLAG_BC;
-    ctx->hflags |= MIPS_HFLAG_BDS32;
 }
 
 static void gen_msa_i8(DisasContext *ctx)
-- 
2.26.2

[PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Philippe Mathieu-Daudé]

In preparation of using the decodetree script, explode
gen_msa_branch() as following:

- OPC_BZ_V              -> BxZ_V(EQ)
- OPC_BNZ_V             -> BxZ_V(NE)
- OPC_BZ_[BHWD]         -> BxZ(false)
- OPC_BNZ_[BHWD]        -> BxZ(true)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 71 ++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 22 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 5311e6ced62..8a35d4d0d03 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28744,49 +28744,76 @@ static void gen_check_zero_element(TCGv tresult, uint8_t df, uint8_t wt)
     tcg_temp_free_i64(t1);
 }
 
+static bool gen_msa_BxZ_V(DisasContext *ctx, int wt, int s16, TCGCond cond)
+{
+    TCGv_i64 t0;
+
+    check_msa_access(ctx);
+
+    if (ctx->hflags & MIPS_HFLAG_BMASK) {
+        generate_exception_end(ctx, EXCP_RI);
+        return true;
+    }
+    t0 = tcg_temp_new_i64();
+    tcg_gen_or_i64(t0, msa_wr_d[wt << 1], msa_wr_d[(wt << 1) + 1]);
+    tcg_gen_setcondi_i64(cond, t0, t0, 0);
+    tcg_gen_trunc_i64_tl(bcond, t0);
+    tcg_temp_free_i64(t0);
+
+    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
+
+    ctx->hflags |= MIPS_HFLAG_BC;
+    ctx->hflags |= MIPS_HFLAG_BDS32;
+
+    return true;
+}
+
+static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
+{
+    check_msa_access(ctx);
+
+    if (ctx->hflags & MIPS_HFLAG_BMASK) {
+        generate_exception_end(ctx, EXCP_RI);
+        return true;
+    }
+
+    gen_check_zero_element(bcond, df, wt);
+    if (if_not) {
+        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
+    }
+
+    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
+    ctx->hflags |= MIPS_HFLAG_BC;
+    ctx->hflags |= MIPS_HFLAG_BDS32;
+
+    return true;
+}
+
 static void gen_msa_branch(DisasContext *ctx, uint32_t op1)
 {
     uint8_t df = (ctx->opcode >> 21) & 0x3;
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
     int64_t s16 = (int16_t)ctx->opcode;
 
-    check_msa_access(ctx);
-
-    if (ctx->hflags & MIPS_HFLAG_BMASK) {
-        generate_exception_end(ctx, EXCP_RI);
-        return;
-    }
     switch (op1) {
     case OPC_BZ_V:
     case OPC_BNZ_V:
-        {
-            TCGv_i64 t0 = tcg_temp_new_i64();
-            tcg_gen_or_i64(t0, msa_wr_d[wt << 1], msa_wr_d[(wt << 1) + 1]);
-            tcg_gen_setcondi_i64((op1 == OPC_BZ_V) ?
-                    TCG_COND_EQ : TCG_COND_NE, t0, t0, 0);
-            tcg_gen_trunc_i64_tl(bcond, t0);
-            tcg_temp_free_i64(t0);
-        }
+        gen_msa_BxZ_V(ctx, wt, s16, (op1 == OPC_BZ_V) ?
+                                    TCG_COND_EQ : TCG_COND_NE);
         break;
     case OPC_BZ_B:
     case OPC_BZ_H:
     case OPC_BZ_W:
     case OPC_BZ_D:
-        gen_check_zero_element(bcond, df, wt);
+        gen_msa_BxZ(ctx, df, wt, s16, false);
         break;
     case OPC_BNZ_B:
     case OPC_BNZ_H:
     case OPC_BNZ_W:
     case OPC_BNZ_D:
-        gen_check_zero_element(bcond, df, wt);
-        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
+        gen_msa_BxZ(ctx, df, wt, s16, true);
         break;
     }
-
-    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
-
-    ctx->hflags |= MIPS_HFLAG_BC;
-    ctx->hflags |= MIPS_HFLAG_BDS32;
 }
 
 static void gen_msa_i8(DisasContext *ctx)
-- 
2.26.2

Re: [PATCH 0/9] target/mips: Simplify MSA TCG logic

[Jiaxun Yang]

在 2020/12/3 上午2:44, Philippe Mathieu-Daudé 写道:
> I converted MSA opcodes to decodetree. To keep the series
> small I split it in 2, this is the non-decodetree specific
> patches (so non-decodetree experts can review it ;) ).
>
> First we stop using env->insn_flags to check for MSAi
> presence, then we restrict TCG functions to DisasContext*.

Hi Philippe,

For the whole series,
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>


I'm just curious about how would you deal with so many condition flags
with decodetree?

Unlike other ISAs, MIPS have so many flavors, every ISA level (MIPS-III 
R2 R5 R6)
has it's own instructions, and in my understanding decodetree file won't 
generate
these switches. I was trying to do the same thing but soon find out 
we'll have around
20 decodertree for MIPS.

Thanks.

- Jiaxun

>
> Based-on: <20201130102228.2395100-1-f4bug@amsat.org>
> "target/mips: Allow executing MSA instructions on Loongson-3A4000"
>
> Philippe Mathieu-Daudé (9):
>    target/mips: Introduce ase_msa_available() helper
>    target/mips: Simplify msa_reset()
>    target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA
>    target/mips: Simplify MSA TCG logic
>    target/mips: Remove now unused ASE_MSA definition
>    target/mips: Alias MSA vector registers on FPU scalar registers
>    target/mips: Extract msa_translate_init() from mips_tcg_init()
>    target/mips: Remove CPUMIPSState* argument from gen_msa*() methods
>    target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()
>
>   target/mips/internal.h           |   8 +-
>   target/mips/mips-defs.h          |   1 -
>   target/mips/kvm.c                |  12 +-
>   target/mips/translate.c          | 206 ++++++++++++++++++-------------
>   target/mips/translate_init.c.inc |  12 +-
>   5 files changed, 138 insertions(+), 101 deletions(-)
>

Re: [PATCH 0/9] target/mips: Simplify MSA TCG logic

[Jiaxun Yang]

在 2020/12/3 上午2:44, Philippe Mathieu-Daudé 写道:
> I converted MSA opcodes to decodetree. To keep the series
> small I split it in 2, this is the non-decodetree specific
> patches (so non-decodetree experts can review it ;) ).
>
> First we stop using env->insn_flags to check for MSAi
> presence, then we restrict TCG functions to DisasContext*.

Hi Philippe,

For the whole series,
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>


I'm just curious about how would you deal with so many condition flags
with decodetree?

Unlike other ISAs, MIPS have so many flavors, every ISA level (MIPS-III 
R2 R5 R6)
has it's own instructions, and in my understanding decodetree file won't 
generate
these switches. I was trying to do the same thing but soon find out 
we'll have around
20 decodertree for MIPS.

Thanks.

- Jiaxun

>
> Based-on: <20201130102228.2395100-1-f4bug@amsat.org>
> "target/mips: Allow executing MSA instructions on Loongson-3A4000"
>
> Philippe Mathieu-Daudé (9):
>    target/mips: Introduce ase_msa_available() helper
>    target/mips: Simplify msa_reset()
>    target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA
>    target/mips: Simplify MSA TCG logic
>    target/mips: Remove now unused ASE_MSA definition
>    target/mips: Alias MSA vector registers on FPU scalar registers
>    target/mips: Extract msa_translate_init() from mips_tcg_init()
>    target/mips: Remove CPUMIPSState* argument from gen_msa*() methods
>    target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()
>
>   target/mips/internal.h           |   8 +-
>   target/mips/mips-defs.h          |   1 -
>   target/mips/kvm.c                |  12 +-
>   target/mips/translate.c          | 206 ++++++++++++++++++-------------
>   target/mips/translate_init.c.inc |  12 +-
>   5 files changed, 138 insertions(+), 101 deletions(-)
>

Re: [PATCH 0/9] target/mips: Simplify MSA TCG logic

[Jiaxun Yang]

在 2020/12/3 上午2:44, Philippe Mathieu-Daudé 写道:
> I converted MSA opcodes to decodetree. To keep the series
> small I split it in 2, this is the non-decodetree specific
> patches (so non-decodetree experts can review it ;) ).
>
> First we stop using env->insn_flags to check for MSAi
> presence, then we restrict TCG functions to DisasContext*.

Hi Philippe,

For the whole series,
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>


I'm just curious about how would you deal with so many condition flags
with decodetree?

Unlike other ISAs, MIPS have so many flavors, every ISA level (MIPS-III 
R2 R5 R6)
has it's own instructions, and in my understanding decodetree file won't 
generate
these switches. I was trying to do the same thing but soon find out 
we'll have around
20 decodetree for MIPS.

Thanks.

- Jiaxun

>
> Based-on: <20201130102228.2395100-1-f4bug@amsat.org>
> "target/mips: Allow executing MSA instructions on Loongson-3A4000"
>
> Philippe Mathieu-Daudé (9):
>    target/mips: Introduce ase_msa_available() helper
>    target/mips: Simplify msa_reset()
>    target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA
>    target/mips: Simplify MSA TCG logic
>    target/mips: Remove now unused ASE_MSA definition
>    target/mips: Alias MSA vector registers on FPU scalar registers
>    target/mips: Extract msa_translate_init() from mips_tcg_init()
>    target/mips: Remove CPUMIPSState* argument from gen_msa*() methods
>    target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()
>
>   target/mips/internal.h           |   8 +-
>   target/mips/mips-defs.h          |   1 -
>   target/mips/kvm.c                |  12 +-
>   target/mips/translate.c          | 206 ++++++++++++++++++-------------
>   target/mips/translate_init.c.inc |  12 +-
>   5 files changed, 138 insertions(+), 101 deletions(-)
>

Re: [PATCH 0/9] target/mips: Simplify MSA TCG logic

[Jiaxun Yang]

在 2020/12/3 上午2:44, Philippe Mathieu-Daudé 写道:
> I converted MSA opcodes to decodetree. To keep the series
> small I split it in 2, this is the non-decodetree specific
> patches (so non-decodetree experts can review it ;) ).
>
> First we stop using env->insn_flags to check for MSAi
> presence, then we restrict TCG functions to DisasContext*.

Hi Philippe,

For the whole series,
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>


I'm just curious about how would you deal with so many condition flags
with decodetree?

Unlike other ISAs, MIPS have so many flavors, every ISA level (MIPS-III 
R2 R5 R6)
has it's own instructions, and in my understanding decodetree file won't 
generate
these switches. I was trying to do the same thing but soon find out 
we'll have around
20 decodetree for MIPS.

Thanks.

- Jiaxun

>
> Based-on: <20201130102228.2395100-1-f4bug@amsat.org>
> "target/mips: Allow executing MSA instructions on Loongson-3A4000"
>
> Philippe Mathieu-Daudé (9):
>    target/mips: Introduce ase_msa_available() helper
>    target/mips: Simplify msa_reset()
>    target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA
>    target/mips: Simplify MSA TCG logic
>    target/mips: Remove now unused ASE_MSA definition
>    target/mips: Alias MSA vector registers on FPU scalar registers
>    target/mips: Extract msa_translate_init() from mips_tcg_init()
>    target/mips: Remove CPUMIPSState* argument from gen_msa*() methods
>    target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()
>
>   target/mips/internal.h           |   8 +-
>   target/mips/mips-defs.h          |   1 -
>   target/mips/kvm.c                |  12 +-
>   target/mips/translate.c          | 206 ++++++++++++++++++-------------
>   target/mips/translate_init.c.inc |  12 +-
>   5 files changed, 138 insertions(+), 101 deletions(-)
>

Re: [PATCH 1/9] target/mips: Introduce ase_msa_available() helper

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Instead of accessing CP0_Config3 directly and checking
> the 'MSA Present' bit, introduce an explicit helper,
> making the code easier to read.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/internal.h  |  6 ++++++
>  target/mips/kvm.c       | 12 ++++++------
>  target/mips/translate.c |  8 +++-----
>  3 files changed, 15 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 1/9] target/mips: Introduce ase_msa_available() helper

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Instead of accessing CP0_Config3 directly and checking
> the 'MSA Present' bit, introduce an explicit helper,
> making the code easier to read.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/internal.h  |  6 ++++++
>  target/mips/kvm.c       | 12 ++++++------
>  target/mips/translate.c |  8 +++-----
>  3 files changed, 15 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/9] target/mips: Simplify msa_reset()

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Call msa_reset() inconditionally, but only reset

unconditionally.

> the MSA registers if MSA is implemented.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Maybe not very useful.

Yeah, it's marginal, especially given one user.
But whichever way you prefer.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/9] target/mips: Simplify msa_reset()

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Call msa_reset() inconditionally, but only reset

unconditionally.

> the MSA registers if MSA is implemented.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Maybe not very useful.

Yeah, it's marginal, especially given one user.
But whichever way you prefer.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/9] target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> MSA presence is expressed by the MSAP bit of CP0_Config3.
> We don't need to check anything else.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/internal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/9] target/mips: Use CP0_Config3 to set MIPS_HFLAG_MSA

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> MSA presence is expressed by the MSAP bit of CP0_Config3.
> We don't need to check anything else.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/internal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 4/9] target/mips: Simplify MSA TCG logic

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Only decode MSA opcodes if MSA is present (implemented).
> 
> Now than check_msa_access() will only be called if MSA is
> present, the only way to have MIPS_HFLAG_MSA unset is if
> MSA is disabled (bit CP0C5_MSAEn cleared, see previous
> commit). Therefore we can remove the 'reserved instruction'
> exception.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/translate.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 4/9] target/mips: Simplify MSA TCG logic

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Only decode MSA opcodes if MSA is present (implemented).
> 
> Now than check_msa_access() will only be called if MSA is
> present, the only way to have MIPS_HFLAG_MSA unset is if
> MSA is disabled (bit CP0C5_MSAEn cleared, see previous
> commit). Therefore we can remove the 'reserved instruction'
> exception.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/translate.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 5/9] target/mips: Remove now unused ASE_MSA definition

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> We don't use ASE_MSA anymore (replaced by ase_msa_available()
> checking MSAP bit from CP0_Config3). Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/mips-defs.h          | 1 -
>  target/mips/translate_init.c.inc | 8 ++++----
>  2 files changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 5/9] target/mips: Remove now unused ASE_MSA definition

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> We don't use ASE_MSA anymore (replaced by ase_msa_available()
> checking MSAP bit from CP0_Config3). Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/mips-defs.h          | 1 -
>  target/mips/translate_init.c.inc | 8 ++++----
>  2 files changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Commits 863f264d10f ("add msa_reset(), global msa register") and
> cb269f273fd ("fix multiple TCG registers covering same data")
> removed the FPU scalar registers and replaced them by aliases to
> the MSA vector registers.
> While this might be the case for CPU implementing MSA, this makes
> QEMU code incoherent for CPU not implementing it. It is simpler
> to inverse the logic and alias the MSA vector registers on the
> FPU scalar ones.

How does it make things incoherent?  I'm missing how the logic has actually
changed, as opposed to an order of assignments.


r~

Re: [PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Commits 863f264d10f ("add msa_reset(), global msa register") and
> cb269f273fd ("fix multiple TCG registers covering same data")
> removed the FPU scalar registers and replaced them by aliases to
> the MSA vector registers.
> While this might be the case for CPU implementing MSA, this makes
> QEMU code incoherent for CPU not implementing it. It is simpler
> to inverse the logic and alias the MSA vector registers on the
> FPU scalar ones.

How does it make things incoherent?  I'm missing how the logic has actually
changed, as opposed to an order of assignments.


r~

Re: [PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Extract the logic initialization of the MSA registers from
> the generic initialization.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/translate.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)

Why?

> -        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
> +        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);

Maybe fold this back to the previous patch?


r~

Re: [PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> Extract the logic initialization of the MSA registers from
> the generic initialization.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/translate.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)

Why?

> -        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
> +        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);

Maybe fold this back to the previous patch?


r~

Re: [PATCH 8/9] target/mips: Remove CPUMIPSState* argument from gen_msa*() methods

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> The gen_msa*() methods don't use the "CPUMIPSState *env"
> argument. Remove it to simplify.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/translate.c | 57 ++++++++++++++++++++---------------------
>  1 file changed, 28 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 8/9] target/mips: Remove CPUMIPSState* argument from gen_msa*() methods

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> The gen_msa*() methods don't use the "CPUMIPSState *env"
> argument. Remove it to simplify.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/translate.c | 57 ++++++++++++++++++++---------------------
>  1 file changed, 28 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> +static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
> +{
> +    check_msa_access(ctx);
> +
> +    if (ctx->hflags & MIPS_HFLAG_BMASK) {
> +        generate_exception_end(ctx, EXCP_RI);
> +        return true;
> +    }
> +
> +    gen_check_zero_element(bcond, df, wt);
> +    if (if_not) {
> +        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
> +    }

Since gen_check_zero_element already produces a boolean, this is better as

  tcg_gen_xori_tl(bcond, bcond, if_not);

where tcg_gen_xori_tl already contains the if.

>      case OPC_BNZ_D:
> -        gen_check_zero_element(bcond, df, wt);
> -        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
> +        gen_msa_BxZ(ctx, df, wt, s16, true);

... oops, that'd be for a follow-up patch, to make this patch just code movement.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Richard Henderson]

On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> +static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
> +{
> +    check_msa_access(ctx);
> +
> +    if (ctx->hflags & MIPS_HFLAG_BMASK) {
> +        generate_exception_end(ctx, EXCP_RI);
> +        return true;
> +    }
> +
> +    gen_check_zero_element(bcond, df, wt);
> +    if (if_not) {
> +        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
> +    }

Since gen_check_zero_element already produces a boolean, this is better as

  tcg_gen_xori_tl(bcond, bcond, if_not);

where tcg_gen_xori_tl already contains the if.

>      case OPC_BNZ_D:
> -        gen_check_zero_element(bcond, df, wt);
> -        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
> +        gen_msa_BxZ(ctx, df, wt, s16, true);

... oops, that'd be for a follow-up patch, to make this patch just code movement.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Philippe Mathieu-Daudé]

On 12/4/20 5:30 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> Extract the logic initialization of the MSA registers from
>> the generic initialization.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  target/mips/translate.c | 35 ++++++++++++++++++++---------------
>>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> Why?

msa_wr_d[] registers are only used by MSA, so in the next series
that allows me to move the 'static msa_wr_d[]' in msa_translate.c,
without having to declare them global with extern.

> 
>> -        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
>> +        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
> 
> Maybe fold this back to the previous patch?

Certainly ;)

> 
> 
> r~
> 

Re: [PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Philippe Mathieu-Daudé]

On 12/4/20 5:30 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> Extract the logic initialization of the MSA registers from
>> the generic initialization.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  target/mips/translate.c | 35 ++++++++++++++++++++---------------
>>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> Why?

msa_wr_d[] registers are only used by MSA, so in the next series
that allows me to move the 'static msa_wr_d[]' in msa_translate.c,
without having to declare them global with extern.

> 
>> -        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, msaregnames[i * 2]);
>> +        fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
> 
> Maybe fold this back to the previous patch?

Certainly ;)

> 
> 
> r~
> 

Re: [PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Richard Henderson]

On 12/4/20 11:23 AM, Philippe Mathieu-Daudé wrote:
> On 12/4/20 5:30 PM, Richard Henderson wrote:
>> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>>> Extract the logic initialization of the MSA registers from
>>> the generic initialization.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  target/mips/translate.c | 35 ++++++++++++++++++++---------------
>>>  1 file changed, 20 insertions(+), 15 deletions(-)
>>
>> Why?
> 
> msa_wr_d[] registers are only used by MSA, so in the next series
> that allows me to move the 'static msa_wr_d[]' in msa_translate.c,
> without having to declare them global with extern.

Ah, sure.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 7/9] target/mips: Extract msa_translate_init() from mips_tcg_init()

[Richard Henderson]

On 12/4/20 11:23 AM, Philippe Mathieu-Daudé wrote:
> On 12/4/20 5:30 PM, Richard Henderson wrote:
>> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>>> Extract the logic initialization of the MSA registers from
>>> the generic initialization.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  target/mips/translate.c | 35 ++++++++++++++++++++---------------
>>>  1 file changed, 20 insertions(+), 15 deletions(-)
>>
>> Why?
> 
> msa_wr_d[] registers are only used by MSA, so in the next series
> that allows me to move the 'static msa_wr_d[]' in msa_translate.c,
> without having to declare them global with extern.

Ah, sure.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Philippe Mathieu-Daudé]

On 12/4/20 5:28 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> Commits 863f264d10f ("add msa_reset(), global msa register") and
>> cb269f273fd ("fix multiple TCG registers covering same data")
>> removed the FPU scalar registers and replaced them by aliases to
>> the MSA vector registers.
>> While this might be the case for CPU implementing MSA, this makes
>> QEMU code incoherent for CPU not implementing it. It is simpler
>> to inverse the logic and alias the MSA vector registers on the
>> FPU scalar ones.
> 
> How does it make things incoherent?  I'm missing how the logic has actually
> changed, as opposed to an order of assignments.

I guess my wording isn't clear.

By "incoherent" I want to say it is odd to disable MSA and have
FPU registers displayed with MSA register names, instead of their
proper FPU names.

The MIPS ISA represents the ASE as onion rings that extend an ISA.
I'd like to model it that way, have ASE optional (and that we can
even not compile).
You can have CPU without FPU, CPU with FPU, CPU with MSA (you
implicitly have a FPU). If FPU depends on MSA, we can not take the
MSA implementation out of the equation.

Back to the patch, instead of aliasing FPU registers to the MSA ones
(even when MSA is absent), we now alias the MSA ones to the FPU ones
(only when MSA is present). This is what I call the "inverted logic".

BTW the point of this change is simply to be able to extract the MSA
code out of the huge translate.c.

Regards,

Phil.

Re: [PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Philippe Mathieu-Daudé]

On 12/4/20 5:28 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> Commits 863f264d10f ("add msa_reset(), global msa register") and
>> cb269f273fd ("fix multiple TCG registers covering same data")
>> removed the FPU scalar registers and replaced them by aliases to
>> the MSA vector registers.
>> While this might be the case for CPU implementing MSA, this makes
>> QEMU code incoherent for CPU not implementing it. It is simpler
>> to inverse the logic and alias the MSA vector registers on the
>> FPU scalar ones.
> 
> How does it make things incoherent?  I'm missing how the logic has actually
> changed, as opposed to an order of assignments.

I guess my wording isn't clear.

By "incoherent" I want to say it is odd to disable MSA and have
FPU registers displayed with MSA register names, instead of their
proper FPU names.

The MIPS ISA represents the ASE as onion rings that extend an ISA.
I'd like to model it that way, have ASE optional (and that we can
even not compile).
You can have CPU without FPU, CPU with FPU, CPU with MSA (you
implicitly have a FPU). If FPU depends on MSA, we can not take the
MSA implementation out of the equation.

Back to the patch, instead of aliasing FPU registers to the MSA ones
(even when MSA is absent), we now alias the MSA ones to the FPU ones
(only when MSA is present). This is what I call the "inverted logic".

BTW the point of this change is simply to be able to extract the MSA
code out of the huge translate.c.

Regards,

Phil.

Re: [PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Philippe Mathieu-Daudé]

On 12/4/20 6:04 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> +static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
>> +{
>> +    check_msa_access(ctx);
>> +
>> +    if (ctx->hflags & MIPS_HFLAG_BMASK) {
>> +        generate_exception_end(ctx, EXCP_RI);
>> +        return true;
>> +    }
>> +
>> +    gen_check_zero_element(bcond, df, wt);
>> +    if (if_not) {
>> +        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
>> +    }
> 
> Since gen_check_zero_element already produces a boolean, this is better as
> 
>   tcg_gen_xori_tl(bcond, bcond, if_not);
> 
> where tcg_gen_xori_tl already contains the if.

Ah, got it.

> 
>>      case OPC_BNZ_D:
>> -        gen_check_zero_element(bcond, df, wt);
>> -        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
>> +        gen_msa_BxZ(ctx, df, wt, s16, true);
> 
> ... oops, that'd be for a follow-up patch, to make this patch just code movement.

Yes, will follow. I'm tempted to inline gen_check_zero_element (actually
move gen_msa_BxZ as gen_check_zero_element prologue/epilogue). Not sure
gen_check_zero_element() can be reused later though.

> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

> 
> r~
> 

Re: [PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Philippe Mathieu-Daudé]

On 12/4/20 6:04 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> +static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
>> +{
>> +    check_msa_access(ctx);
>> +
>> +    if (ctx->hflags & MIPS_HFLAG_BMASK) {
>> +        generate_exception_end(ctx, EXCP_RI);
>> +        return true;
>> +    }
>> +
>> +    gen_check_zero_element(bcond, df, wt);
>> +    if (if_not) {
>> +        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
>> +    }
> 
> Since gen_check_zero_element already produces a boolean, this is better as
> 
>   tcg_gen_xori_tl(bcond, bcond, if_not);
> 
> where tcg_gen_xori_tl already contains the if.

Ah, got it.

> 
>>      case OPC_BNZ_D:
>> -        gen_check_zero_element(bcond, df, wt);
>> -        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
>> +        gen_msa_BxZ(ctx, df, wt, s16, true);
> 
> ... oops, that'd be for a follow-up patch, to make this patch just code movement.

Yes, will follow. I'm tempted to inline gen_check_zero_element (actually
move gen_msa_BxZ as gen_check_zero_element prologue/epilogue). Not sure
gen_check_zero_element() can be reused later though.

> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

> 
> r~
> 

Re: [PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Richard Henderson]

On 12/4/20 4:40 PM, Philippe Mathieu-Daudé wrote:
> Back to the patch, instead of aliasing FPU registers to the MSA ones
> (even when MSA is absent), we now alias the MSA ones to the FPU ones
> (only when MSA is present). This is what I call the "inverted logic".
> 
> BTW the point of this change is simply to be able to extract the MSA
> code out of the huge translate.c.

Yes, I see that at the end of the series.  Have a

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 6/9] target/mips: Alias MSA vector registers on FPU scalar registers

[Richard Henderson]

On 12/4/20 4:40 PM, Philippe Mathieu-Daudé wrote:
> Back to the patch, instead of aliasing FPU registers to the MSA ones
> (even when MSA is absent), we now alias the MSA ones to the FPU ones
> (only when MSA is present). This is what I call the "inverted logic".
> 
> BTW the point of this change is simply to be able to extract the MSA
> code out of the huge translate.c.

Yes, I see that at the end of the series.  Have a

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Richard Henderson]

On 12/4/20 4:53 PM, Philippe Mathieu-Daudé wrote:
> Yes, will follow. I'm tempted to inline gen_check_zero_element (actually
> move gen_msa_BxZ as gen_check_zero_element prologue/epilogue). Not sure
> gen_check_zero_element() can be reused later though.

The other thing that could happen is that gen_check_zero_element could grow a
TCGCond argument (or boolean) for the setcond at the end, so that we generate
the correct sense of the test in the first place.


r~

Re: [PATCH 9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

[Richard Henderson]

On 12/4/20 4:53 PM, Philippe Mathieu-Daudé wrote:
> Yes, will follow. I'm tempted to inline gen_check_zero_element (actually
> move gen_msa_BxZ as gen_check_zero_element prologue/epilogue). Not sure
> gen_check_zero_element() can be reused later though.

The other thing that could happen is that gen_check_zero_element could grow a
TCGCond argument (or boolean) for the setcond at the end, so that we generate
the correct sense of the test in the first place.


r~