On 3/27/23 20:05, Song Gao wrote:
+#define DO_ODD_EVEN_S(NAME, BIT, T, E1, E2, DO_OP)                 \
+void HELPER(NAME)(CPULoongArchState *env,                          \
+                  uint32_t vd, uint32_t vj, uint32_t vk)           \
+{                                                                  \
+    int i;                                                         \
+    VReg *Vd = &(env->fpr[vd].vreg);                               \
+    VReg *Vj = &(env->fpr[vj].vreg);                               \
+    VReg *Vk = &(env->fpr[vk].vreg);                               \
+                                                                   \
+    for (i = 0; i < LSX_LEN/BIT; i++) {                            \
+        Vd->E1(i) = DO_OP((T)Vj->E2(2 * i + 1), (T)Vk->E2(2 * i)); \
+    }                                                              \
+}
...
+#define DO_ODD_EVEN_U(NAME, BIT, TD, TS,  E1, E2, DO_OP)                     \
+void HELPER(NAME)(CPULoongArchState *env,                                    \
+                  uint32_t vd, uint32_t vj, uint32_t vk)                     \
+{                                                                            \
+    int i;                                                                   \
+    VReg *Vd = &(env->fpr[vd].vreg);                                         \
+    VReg *Vj = &(env->fpr[vj].vreg);                                         \
+    VReg *Vk = &(env->fpr[vk].vreg);                                         \
+                                                                             \
+    for (i = 0; i < LSX_LEN/BIT; i++) {                                      \
+        Vd->E1(i) = DO_OP((TD)(TS)Vj->E2(2 * i + 1), (TD)(TS)Vk->E2(2 * i)); \
+    }                                                                        \
+}

In the first case we have one cast, in the second case we have two. I wonder if it would be clearer to have both signed and unsigned members in the VReg union? Then these two macros could be combined.

I also think we could make use of (__typeof(Vd->E1(0))) instead of separately passing the output type? It would appear to be less error-prone.

All that said, the code as written is correct so,
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>


r~

Reply via email to