Re: [RFC PATCH v2 09/44] target/loongarch: Implement vhaddw/vhsubw

[Richard Henderson]

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.henderson@linaro.org>


r~