diff mbox series

[1/7] target/mips: Add MXU register support

Message ID 261dd8062c85c2a5eefb4d6effa2a44d5fc953f7.1535133089.git.jancraig@amazon.com (mailing list archive)
State New, archived
Headers show
Series Add limited MXU instruction support | expand

Commit Message

Gonglei (Arei)" via Aug. 24, 2018, 7:44 p.m. UTC
This commit makes the MXU registers and the helper functions for
reading/writing to them. This is required for full MXU instruction
support.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/cpu.h       |  1 +
 target/mips/translate.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

Comments

Richard Henderson Aug. 25, 2018, 4:50 p.m. UTC | #1
On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> +/* MXU General purpose registers moves. */
> +static inline void gen_load_mxu_gpr (TCGv t, int reg)
> +{
> +    if (reg == 0)
> +        tcg_gen_movi_tl(t, 0);
> +    else
> +        tcg_gen_mov_tl(t, mxu_gpr[reg-1]);
> +}
> +
> +static inline void gen_store_mxu_gpr (TCGv t, int reg)
> +{
> +    if (reg != 0)
> +        tcg_gen_mov_tl(mxu_gpr[reg-1], t);
> +}
> +
>  /* Moves to/from shadow registers. */
>  static inline void gen_load_srsgpr (int from, int to)
>  {
> @@ -20742,6 +20767,11 @@ void mips_tcg_init(void)
>      fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
>                                         offsetof(CPUMIPSState, active_fpu.fcr31),
>                                         "fcr31");
> +
> +    for (i = 0; i < 16; i++)
> +        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
> +                                        offsetof(CPUMIPSState, active_tc.mxu_gpr[i]),
> +                                        mxuregnames[i]);
>  }

You need to fix the ./scripts/checkpatch.pl errors.
But otherwise the logic is ok.


r~
Aleksandar Markovic Aug. 27, 2018, 12:35 p.m. UTC | #2
> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Friday, August 24, 2018 9:44 PM
>
> Subject: [PATCH 1/7] target/mips: Add MXU register support
> 
> This commit makes the MXU registers and the helper functions for
> reading/writing to them. This is required for full MXU instruction
> support.

Hi, Craig,

The term "helper function" is good in general terminology sense, however, in QEMU terminology, it is used for something else (see op_helper.c for examples), and not for the case similar to the functions in this patch. Your functions generate "inline" code, in QEMU terminology, which is opposite to "helper" functions. Just don't use the word "helper" in the commit message. Use "wrapper", "utility", or similar word.

Thanks,
Aleksandar
Aleksandar Markovic Aug. 27, 2018, 12:41 p.m. UTC | #3
> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Friday, August 24, 2018 9:44 PM
>
> Subject: [PATCH 1/7] target/mips: Add MXU register support
> 
>This commit makes the MXU registers and the helper functions for
> reading/writing to them. This is required for full MXU instruction
> support.

> Signed-off-by: Craig Janeczek <jancraig@amazon.com>

Also, Craig,

This patch will cause compiler errors if QEMU is built with clang, with this patch as the last one. Clang will complain about unused functions. This will affect bisect algorithms. you should move definition of the functions to the patches where they are used for the first time in the series.

Otherwise, overall, we want the content of your series integrated, you just need to improve/correct the code.

Regards,
Aleksandar
diff mbox series

Patch

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 009202cf64..4b2948a2c8 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -170,6 +170,7 @@  struct TCState {
         MSACSR_FS_MASK)
 
     float_status msa_fp_status;
+    target_ulong mxu_gpr[16];
 };
 
 typedef struct CPUMIPSState CPUMIPSState;
diff --git a/target/mips/translate.c b/target/mips/translate.c
index bdd880bb77..50f0cb558f 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1398,6 +1398,9 @@  static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
 
+/* MXU registers */
+static TCGv mxu_gpr[16];
+
 #include "exec/gen-icount.h"
 
 #define gen_helper_0e0i(name, arg) do {                           \
@@ -1517,6 +1520,13 @@  static const char * const msaregnames[] = {
     "w30.d0", "w30.d1", "w31.d0", "w31.d1",
 };
 
+static const char * const mxuregnames[] = {
+    "XR1",  "XR2",  "XR3",  "XR4",  "XR5",
+    "XR6",  "XR7",  "XR8",  "XR9",  "XR10",
+    "XR11", "XR12", "XR13", "XR14", "XR15",
+    "XR16",
+};
+
 #define LOG_DISAS(...)                                                        \
     do {                                                                      \
         if (MIPS_DEBUG_DISAS) {                                               \
@@ -1550,6 +1560,21 @@  static inline void gen_store_gpr (TCGv t, int reg)
         tcg_gen_mov_tl(cpu_gpr[reg], t);
 }
 
+/* MXU General purpose registers moves. */
+static inline void gen_load_mxu_gpr (TCGv t, int reg)
+{
+    if (reg == 0)
+        tcg_gen_movi_tl(t, 0);
+    else
+        tcg_gen_mov_tl(t, mxu_gpr[reg-1]);
+}
+
+static inline void gen_store_mxu_gpr (TCGv t, int reg)
+{
+    if (reg != 0)
+        tcg_gen_mov_tl(mxu_gpr[reg-1], t);
+}
+
 /* Moves to/from shadow registers. */
 static inline void gen_load_srsgpr (int from, int to)
 {
@@ -20742,6 +20767,11 @@  void mips_tcg_init(void)
     fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
+
+    for (i = 0; i < 16; i++)
+        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
+                                        offsetof(CPUMIPSState, active_tc.mxu_gpr[i]),
+                                        mxuregnames[i]);
 }
 
 #include "translate_init.inc.c"