Re: [PATCH v4 02/11] target/loongarch: Add new object class for loongarch32 cpus

[Richard Henderson]

On 8/7/23 18:54, Jiajie Chen wrote:
Add object class for future loongarch32 cpus. It is derived from the
loongarch64 object class.
Signed-off-by: Jiajie Chen <c...@jia.je>
---
  target/loongarch/cpu.c | 24 ++++++++++++++++++++++++
  target/loongarch/cpu.h | 11 +++++++++++
  2 files changed, 35 insertions(+)

diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index ad93ecac92..3bd293d00a 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -732,6 +732,10 @@ static void loongarch_cpu_class_init(ObjectClass *c, void 
*data)
  #endif
  }
  
+static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
+{
+}
+
  #define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
      { \
          .parent = TYPE_LOONGARCH_CPU, \
@@ -754,3 +758,23 @@ static const TypeInfo loongarch_cpu_type_infos[] = {
  };
  
  DEFINE_TYPES(loongarch_cpu_type_infos)
+
+#define DEFINE_LOONGARCH32_CPU_TYPE(model, initfn) \
+    { \
+        .parent = TYPE_LOONGARCH32_CPU, \
+        .instance_init = initfn, \
+        .name = LOONGARCH_CPU_TYPE_NAME(model), \
+    }
+
+static const TypeInfo loongarch32_cpu_type_infos[] = {
+    {
+        .name = TYPE_LOONGARCH32_CPU,
+        .parent = TYPE_LOONGARCH_CPU,
+        .instance_size = sizeof(LoongArchCPU),
+
+        .abstract = true,
+        .class_size = sizeof(LoongArchCPUClass),
+        .class_init = loongarch32_cpu_class_init,
+    },
+};

You don't need to create a new array, you can put these into the existing 
loongarch_cpu_type_infos[] like so:


static const TypeInfo loongarch_cpu_type_infos[] = {
    {
        .name = TYPE_LOONGARCH_CPU,
        ...
    },
    {
        .name = TYPE_LOONGARCH32_CPU,
        ...
    },
    DEFINE_LOONGARCH_CPU_TYPE("la464", loongarch_la464_initfn),
    DEFINE_LOONGARCH32_CPU_TYPE("la132", loongarch_la132_initfn),
};


+#define TYPE_LOONGARCH32_CPU "loongarch32-cpu"
+typedef struct LoongArch32CPUClass LoongArch32CPUClass;
+DECLARE_CLASS_CHECKERS(LoongArch32CPUClass, LOONGARCH32_CPU,
+                       TYPE_LOONGARCH32_CPU)
+
+struct LoongArch32CPUClass {
+    /*< private >*/
+    LoongArchCPUClass parent_class;
+    /*< public >*/
+};

You don't need to declare another struct if it's just a wrapper.
If you do declare another struct, then you must actually use it in the .class_size 
initializer.


Also, I've noticed two existing bugs:

(1) Missing alignment on fprs, which is required by tcg_gen_gvec_*:

  typedef struct CPUArchState {
      uint64_t gpr[32];
      uint64_t pc;

-     fpr_t fpr[32];
+     fpr_t fpr[32] QEMU_ALIGNED(16);


(2) Missing instance_align on the class:

    .instance_align = __alignof(LoongArchCPU),


r~