Re: [PATCH v2 09/46] target/loongarch: Implement xvhaddw/xvhsubw

[Richard Henderson]

On 6/30/23 09:58, Song Gao wrote:
--- a/target/loongarch/lsx_helper.c
+++ b/target/loongarch/vec_helper.c
@@ -14,20 +14,18 @@
  #include "tcg/tcg.h"
  #include "vec.h"
  
-#define DO_ADD(a, b)  (a + b)
-#define DO_SUB(a, b)  (a - b)
-
  #define DO_ODD_EVEN(NAME, BIT, E1, E2, DO_OP)                        \
-void HELPER(NAME)(CPULoongArchState *env,                            \
+void HELPER(NAME)(CPULoongArchState *env, uint32_t oprsz,            \
                    uint32_t vd, uint32_t vj, uint32_t vk)             \
  {                                                                    \
-    int i;                                                           \
+    int i, len;                                                      \
      VReg *Vd = &(env->fpr[vd].vreg);                                 \
      VReg *Vj = &(env->fpr[vj].vreg);                                 \
      VReg *Vk = &(env->fpr[vk].vreg);                                 \
      typedef __typeof(Vd->E1(0)) TD;                                  \
                                                                       \
-    for (i = 0; i < LSX_LEN/BIT; i++) {                              \
+    len = (oprsz == 16) ? LSX_LEN : LASX_LEN;                        \
+    for (i = 0; i < len / BIT ; i++) {                               \
          Vd->E1(i) = DO_OP((TD)Vj->E2(2 * i + 1), (TD)Vk->E2(2 * i)); \
      }                                                                \
  }

It would be better to use the gen_helper_gvec_3 function signature:

void HELPER(name)(void *vd, void *vj, void *vk, uint32_t desc)
{
    VReg *Vd = vd, ...;
    int oprsz = simd_oprsz(desc);

    for (i = 0; i < oprsz / (BIT / 8); i++) {
        ...
    }
}

You should do the file rename and the conversion of the existing LSX operations in a 
separate patch.

@@ -44,13 +42,17 @@ void HELPER(vhaddw_q_d)(CPULoongArchState *env,
      VReg *Vk = &(env->fpr[vk].vreg);
  
      Vd->Q(0) = int128_add(int128_makes64(Vj->D(1)), int128_makes64(Vk->D(0)));
+    if (oprsz == 32) {
+        Vd->Q(1) = int128_add(int128_makes64(Vj->D(3)),
+                              int128_makes64(Vk->D(2)));
+    }

Better as a loop, and all the rest.


r~