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

[Richard Henderson]

On 10/10/22 12:13, Lucas Mateus Castro(alqotel) wrote:
> +/* test if +Inf or -Inf */
> +static void gen_is_any_inf(unsigned vece, TCGv_vec t, TCGv_vec b)
> +{
> +    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;
> +    tcg_gen_andc_vec(vece, b, b, tcg_constant_vec_matching(t, vece, exp_msk));
> +    tcg_gen_cmp_vec(TCG_COND_EQ, vece, t, b,
> +                    tcg_constant_vec_matching(t, vece, sgn_msk));
> +}

Should be clearing sign and comparing exp, not the other way.

> +    GVecGen2 op = {
> +        .fno = (vece == MO_32) ? gen_helper_XVTSTDCSP : gen_helper_XVTSTDCDP,
> +        .vece = vece,
> +        .opt_opc = vecop_list
>       };
>   
>       REQUIRE_VSX(ctx);
>   
> -    tcg_gen_gvec_2i(vsr_full_offset(a->xt), vsr_full_offset(a->xb),
> -                    16, 16, (int32_t)(a->uim), &op[vece - MO_32]);
> +    switch (a->uim) {
> +    case 0:
> +        set_cpu_vsr(a->xt, tcg_constant_i64(0), true);
> +        set_cpu_vsr(a->xt, tcg_constant_i64(0), false);
> +        break;
> +    case ((1 << 0) | (1 << 1)):
> +        /* test if +Denormal or -Denormal */
> +        op.fniv = gen_is_any_denormal,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);

This default setting of .fno doesn't work, because the helper requires simd_data set, 
which this invocation via tcg_gen_gvec_2 will not provide.

You could fix this by using GVecGen2i and tcg_gen_gvec_2i, and ignoring the immediate 
argument added to the functions above.  Which also means...

> +    case (1 << 0):
> +        /* test if -Denormal */
> +        op.fniv = gen_is_neg_denormal,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case (1 << 1):
> +        /* test if +Denormal */
> +        op.fniv = gen_is_pos_denormal,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case ((1 << 2) | (1 << 3)):
> +        /* test if +0 or -0 */
> +        op.fniv = gen_is_any_zero,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case (1 << 2):
> +        /* test if -0 */
> +        op.fniv = gen_is_neg_zero,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case (1 << 3):
> +        /* test if +0 */
> +        op.fniv = gen_is_pos_zero,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case ((1 << 4) | (1 << 5)):
> +        /* test if +Inf or -Inf */
> +        op.fniv = gen_is_any_inf,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case (1 << 4):
> +        /* test if -Inf */
> +        op.fniv = gen_is_neg_inf,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case (1 << 5):
> +        /* test if +Inf */
> +        op.fniv = gen_is_pos_inf,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    case (1 << 6):
> +        /* test if NaN */
> +        op.fniv = gen_is_nan,
> +        tcg_gen_gvec_2(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16, 16,
> +                       &op);
> +        break;
> +    default:
> +        tcg_gen_gvec_2_ool(vsr_full_offset(a->xt), vsr_full_offset(a->xb), 16,
> +                           16, (int32_t)(a->uim), op.fno);
> +        break;

You can have only the store to op.fniv in the switch, remove the default, and have a 
common call to tcg_gen_gvec_2i after the switch.


r~