Re: [qemu-s390x] [PATCH v2 06/22] s390x/tcg: Implement VECTOR FP COMPARE (EQUAL|HIGH|HIGH OR EQUAL)

[David Hildenbrand]

On 03.06.19 11:06, David Hildenbrand wrote:
> Provide for all three instructions all four combinations of cc bit and
> s bit.
> 
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>  target/s390x/helper.h           |  12 ++++
>  target/s390x/insn-data.def      |   6 ++
>  target/s390x/translate_vx.inc.c |  51 ++++++++++++++++
>  target/s390x/vec_fpu_helper.c   | 104 ++++++++++++++++++++++++++++++++
>  4 files changed, 173 insertions(+)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index d34d6802a6..33d3bacf74 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -254,6 +254,18 @@ DEF_HELPER_FLAGS_5(gvec_vfa64, TCG_CALL_NO_WG, void, 
> ptr, cptr, cptr, env, i32)
>  DEF_HELPER_FLAGS_5(gvec_vfa64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, 
> i32)
>  DEF_HELPER_4(gvec_wfc64, void, cptr, cptr, env, i32)
>  DEF_HELPER_4(gvec_wfk64, void, cptr, cptr, env, i32)
> +DEF_HELPER_FLAGS_5(gvec_vfce64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, 
> i32)
> +DEF_HELPER_FLAGS_5(gvec_vfce64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, 
> i32)
> +DEF_HELPER_5(gvec_vfce64_cc, void, ptr, cptr, cptr, env, i32)
> +DEF_HELPER_5(gvec_vfce64s_cc, void, ptr, cptr, cptr, env, i32)
> +DEF_HELPER_FLAGS_5(gvec_vfch64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, 
> i32)
> +DEF_HELPER_FLAGS_5(gvec_vfch64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, 
> i32)
> +DEF_HELPER_5(gvec_vfch64_cc, void, ptr, cptr, cptr, env, i32)
> +DEF_HELPER_5(gvec_vfch64s_cc, void, ptr, cptr, cptr, env, i32)
> +DEF_HELPER_FLAGS_5(gvec_vfche64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, 
> i32)
> +DEF_HELPER_FLAGS_5(gvec_vfche64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, 
> env, i32)
> +DEF_HELPER_5(gvec_vfche64_cc, void, ptr, cptr, cptr, env, i32)
> +DEF_HELPER_5(gvec_vfche64s_cc, void, ptr, cptr, cptr, env, i32)
>  
>  #ifndef CONFIG_USER_ONLY
>  DEF_HELPER_3(servc, i32, env, i64, i64)
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index c45e101b10..446552f251 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -1212,6 +1212,12 @@
>      F(0xe7cb, WFC,     VRR_a, V,   0, 0, 0, 0, wfc, 0, IF_VEC)
>  /* VECTOR FP COMPARE AND SIGNAL SCALAR */
>      F(0xe7ca, WFK,     VRR_a, V,   0, 0, 0, 0, wfc, 0, IF_VEC)
> +/* VECTOR FP COMPARE EQUAL */
> +    F(0xe7e8, VFCE,    VRR_c, V,   0, 0, 0, 0, vfc, 0, IF_VEC)
> +/* VECTOR FP COMPARE HIGH */
> +    F(0xe7eb, VFCH,    VRR_c, V,   0, 0, 0, 0, vfc, 0, IF_VEC)
> +/* VECTOR FP COMPARE HIGH OR EQUAL */
> +    F(0xe7ea, VFCHE,   VRR_c, V,   0, 0, 0, 0, vfc, 0, IF_VEC)
>  
>  #ifndef CONFIG_USER_ONLY
>  /* COMPARE AND SWAP AND PURGE */
> diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
> index 283e8aa07a..5571a71e1a 100644
> --- a/target/s390x/translate_vx.inc.c
> +++ b/target/s390x/translate_vx.inc.c
> @@ -2588,3 +2588,54 @@ static DisasJumpType op_wfc(DisasContext *s, DisasOps 
> *o)
>      set_cc_static(s);
>      return DISAS_NEXT;
>  }
> +
> +static DisasJumpType op_vfc(DisasContext *s, DisasOps *o)
> +{
> +    const uint8_t fpf = get_field(s->fields, m4);
> +    const uint8_t m5 = get_field(s->fields, m5);
> +    const uint8_t m6 = get_field(s->fields, m6);
> +    const bool se = extract32(m5, 3, 1);
> +    const bool cs = extract32(m6, 0, 1);
> +    gen_helper_gvec_3_ptr *fn;
> +
> +    if (fpf != FPF_LONG || extract32(m5, 0, 3) || extract32(m6, 1, 3)) {
> +        gen_program_exception(s, PGM_SPECIFICATION);
> +        return DISAS_NORETURN;
> +    }
> +
> +    if (cs) {
> +        switch (s->fields->op2) {
> +        case 0xe8:
> +            fn = se ? gen_helper_gvec_vfce64s_cc : gen_helper_gvec_vfce64_cc;
> +            break;
> +        case 0xeb:
> +            fn = se ? gen_helper_gvec_vfch64s_cc : gen_helper_gvec_vfch64_cc;
> +            break;
> +        case 0xea:
> +            fn = se ? gen_helper_gvec_vfche64s_cc : 
> gen_helper_gvec_vfche64_cc;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +    } else {
> +        switch (s->fields->op2) {
> +        case 0xe8:
> +            fn = se ? gen_helper_gvec_vfce64s : gen_helper_gvec_vfce64;
> +            break;
> +        case 0xeb:
> +            fn = se ? gen_helper_gvec_vfch64s : gen_helper_gvec_vfch64;
> +            break;
> +        case 0xea:
> +            fn = se ? gen_helper_gvec_vfche64s : gen_helper_gvec_vfche64;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +    }
> +    gen_gvec_3_ptr(get_field(s->fields, v1), get_field(s->fields, v2),
> +                   get_field(s->fields, v3), cpu_env, 0, fn);
> +    if (cs) {
> +        set_cc_static(s);
> +    }
> +    return DISAS_NEXT;
> +}
> diff --git a/target/s390x/vec_fpu_helper.c b/target/s390x/vec_fpu_helper.c
> index f9357d9221..e72500d4d5 100644
> --- a/target/s390x/vec_fpu_helper.c
> +++ b/target/s390x/vec_fpu_helper.c
> @@ -149,3 +149,107 @@ void HELPER(gvec_wfk64)(const void *v1, const void *v2, 
> CPUS390XState *env,
>  {
>      env->cc_op = wfc64(v1, v2, env, true, GETPC());
>  }
> +
> +typedef int (*vfc64_fn)(float64 a, float64 b, float_status *status);
> +static int vfc64(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
> +                 CPUS390XState *env, bool s, vfc64_fn fn, uintptr_t retaddr)
> +{
> +    uint8_t vxc, vec_exc = 0;
> +    S390Vector tmp = {};
> +    int match = 0;
> +    int i;
> +
> +    for (i = 0; i < 2; i++) {
> +        const float64 a = s390_vec_read_element64(v2, i);
> +        const float64 b = s390_vec_read_element64(v3, i);
> +
> +        /* swap the order of the parameters, so we can use existing 
> functions */
> +        if (fn(b, a, &env->fpu_status)) {
> +            match++;
> +            s390_vec_write_element64(&tmp, i, -1ull);
> +        }
> +        vxc = check_ieee_exc(env, i, false, &vec_exc);
> +        if (s || vxc) {
> +            break;
> +        }
> +    }
> +
> +    handle_ieee_exc(env, vxc, vec_exc, retaddr);
> +    *v1 = tmp;
> +    if (match) {
> +        return s || match == 2 ? 0 : 1;
> +    }
> +    return 3;
> +}
> +
> +void HELPER(gvec_vfce64)(void *v1, const void *v2, const void *v3,
> +                         CPUS390XState *env, uint32_t desc)
> +{
> +    vfc64(v1, v2, v3, env, false, float64_eq_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfce64s)(void *v1, const void *v2, const void *v3,
> +                          CPUS390XState *env, uint32_t desc)
> +{
> +    vfc64(v1, v2, v3, env, true, float64_eq_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfce64_cc)(void *v1, const void *v2, const void *v3,
> +                            CPUS390XState *env, uint32_t desc)
> +{
> +    env->cc_op = vfc64(v1, v2, v3, env, false, float64_eq_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfce64s_cc)(void *v1, const void *v2, const void *v3,
> +                            CPUS390XState *env, uint32_t desc)
> +{
> +    env->cc_op = vfc64(v1, v2, v3, env, true, float64_eq_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfch64)(void *v1, const void *v2, const void *v3,
> +                         CPUS390XState *env, uint32_t desc)
> +{
> +    vfc64(v1, v2, v3, env, false, float64_lt_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfch64s)(void *v1, const void *v2, const void *v3,
> +                          CPUS390XState *env, uint32_t desc)
> +{
> +    vfc64(v1, v2, v3, env, true, float64_lt_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfch64_cc)(void *v1, const void *v2, const void *v3,
> +                            CPUS390XState *env, uint32_t desc)
> +{
> +    env->cc_op = vfc64(v1, v2, v3, env, false, float64_lt_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfch64s_cc)(void *v1, const void *v2, const void *v3,
> +                             CPUS390XState *env, uint32_t desc)
> +{
> +    env->cc_op = vfc64(v1, v2, v3, env, true, float64_lt_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfche64)(void *v1, const void *v2, const void *v3,
> +                          CPUS390XState *env, uint32_t desc)
> +{
> +    vfc64(v1, v2, v3, env, false, float64_le_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfche64s)(void *v1, const void *v2, const void *v3,
> +                           CPUS390XState *env, uint32_t desc)
> +{
> +    vfc64(v1, v2, v3, env, true, float64_le_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfche64_cc)(void *v1, const void *v2, const void *v3,
> +                             CPUS390XState *env, uint32_t desc)
> +{
> +    env->cc_op = vfc64(v1, v2, v3, env, false, float64_le_quiet, GETPC());
> +}
> +
> +void HELPER(gvec_vfche64s_cc)(void *v1, const void *v2, const void *v3,
> +                              CPUS390XState *env, uint32_t desc)
> +{
> +    env->cc_op = vfc64(v1, v2, v3, env, true, float64_le_quiet, GETPC());
> +}
> 

Richard, still want to review this patch? I'm planning to send a pull
request soon.

-- 

Thanks,

David / dhildenb