Re: [PATCH 12/12] target/ppc: Use gvec to decode XVTSTDC[DS]P

[Richard Henderson]

On 9/23/22 21:47, Lucas Mateus Castro(alqotel) wrote:
> +static void do_xvtstdc_vec(unsigned vece, TCGv_vec t, TCGv_vec b, int64_t imm)
> +{
> +    TCGv_vec match = tcg_const_ones_vec_matching(t);
> +    TCGv_vec temp;
> +    TCGv_vec mask;
> +    uint64_t exp_msk = (vece == MO_32) ? (uint32_t)EXP_MASK_SP : EXP_MASK_DP;
> +    uint64_t sgn_msk = (vece == MO_32) ? (uint32_t)SGN_MASK_SP : SGN_MASK_DP;
> +    uint64_t frc_msk = ~(exp_msk | sgn_msk);
> +    mask = tcg_constant_vec_matching(t, vece, 0);
> +    tcg_gen_mov_vec(t, mask);
> +    if (imm & (0x3 << 0)) {
> +        /* test if Denormal */
> +        temp = tcg_temp_new_vec_matching(t);
> +        mask = tcg_constant_vec_matching(t, vece, ~sgn_msk);
> +        tcg_gen_and_vec(vece, t, b, mask);
> +        mask = tcg_constant_vec_matching(t, vece, frc_msk);
> +        tcg_gen_cmp_vec(TCG_COND_LE, vece, temp, t, mask);
> +        mask = tcg_constant_vec_matching(t, vece, 0);
> +        tcg_gen_cmpsel_vec(TCG_COND_NE, vece, temp, t, mask, temp, mask);
> +
> +        tcg_gen_mov_vec(t, mask);
> +        mask = tcg_constant_vec_matching(t, vece, sgn_msk);
> +        if (imm & (0x1)) {
> +            /* test if negative */
> +            tcg_gen_cmpsel_vec(TCG_COND_GTU, vece, t, b, mask, temp, t);
> +        }
> +        if (imm & (0x2)) {
> +            /* test if positive */
> +            tcg_gen_cmpsel_vec(TCG_COND_LTU, vece, t, b, mask, temp, t);
> +        }
> +        tcg_temp_free_vec(temp);
> +    }
> +    if (imm & (1 << 2)) {
> +        /* test if -0 */
> +        mask = tcg_constant_vec_matching(t, vece, sgn_msk);
> +        tcg_gen_cmpsel_vec(TCG_COND_EQ, vece, t, b, mask, match, t);
> +    }
> +    if (imm & (1 << 3)) {
> +        /* test if +0 */
> +        mask = tcg_constant_vec_matching(t, vece, 0);
> +        tcg_gen_cmpsel_vec(TCG_COND_EQ, vece, t, b, mask, match, t);
> +    }
> +    if (imm & (1 << 4)) {
> +        /* test if -Inf */
> +        mask = tcg_constant_vec_matching(t, vece, exp_msk | sgn_msk);
> +        tcg_gen_cmpsel_vec(TCG_COND_EQ, vece, t, b, mask, match, t);
> +    }
> +    if (imm & (1 << 5)) {
> +        /* test if +Inf */
> +        mask = tcg_constant_vec_matching(t, vece, exp_msk);
> +        tcg_gen_cmpsel_vec(TCG_COND_EQ, vece, t, b, mask, match, t);
> +    }
> +    if (imm & (1 << 6)) {
> +        /* test if NaN */
> +        mask = tcg_constant_vec_matching(t, vece, ~sgn_msk);
> +        tcg_gen_and_vec(vece, b, b, mask);
> +        mask = tcg_constant_vec_matching(t, vece, exp_msk);
> +        tcg_gen_cmpsel_vec(TCG_COND_GT, vece, t, b, mask, match, t);
> +    }
> +    tcg_temp_free_vec(match);
> +}

While each case is fairly clever, I don't think that stringing them together like this is 
a good idea.  I think you should only handle the easy cases inline, and defer random (and 
probably rarely used) bit combinations to the helper function.

For instance,

static void gen_is_pos_inf(unsigned vece, TCGv_vec t, TCGv_vec b)
{
    tcg_gen_cmp_vec(TCG_COND_EQ, vece, t, b,
        tcg_constant_vec_matching(t, vece, exp_mask));
}

static void gen_is_any_inf(unsigned vece, TCGv_vec t, TCGv_vec b)
{
    tcg_gen_and_vec(vece, t, b,
        tcg_constant_vec_matching(t, vece, ~sgn_mask));
    tcg_gen_cmp_vec(TCG_COND_EQ, vece, t, b,
        tcg_constant_vec_matching(t, vece, exp_mask));
}

static bool do_xvtstdc(...)
{
    switch (a->imm) {
    case (1 << 4): /* -Inf */
        tcg_gen_gvec_2(..., &op_is_neg_inf);
        break;
    case (1 << 5): /* +Inf */
        tcg_gen_gvec_2(..., &op_is_pos_inf);
        break;
    case (1 << 4) | (1 << 5): /* -Inf | +Inf */
        tcg_gen_gvec_2(..., &op_is_any_inf);
        break;
    ...
    default:
        tcg_gen_gvec_2_ool(..., 16, 16, a->imm, gen_helper_XVTSTDCXX);
    }
}

Or something of that nature.

I'll also note that you don't need CMPSEL -- all cases are mutually exclusive, so OR works 
just as well.

r~