Re: [PATCH v1 08/20] s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE (AND SIGNAL) SCALAR

[Richard Henderson]

On 9/30/20 9:55 AM, David Hildenbrand wrote:
> @@ -2601,19 +2601,41 @@ static DisasJumpType op_wfc(DisasContext *s, DisasOps 
> *o)
>  {
>      const uint8_t fpf = get_field(s, m3);
>      const uint8_t m4 = get_field(s, m4);
> +    gen_helper_gvec_2_ptr *fn = NULL;
>  
> -    if (fpf != FPF_LONG || m4) {
> +    switch (fpf) {
> +    case FPF_SHORT:
> +        if (s390_has_feat(S390_FEAT_VECTOR_ENH)) {
> +            fn = gen_helper_gvec_wfk32;
> +            if (s->fields.op2 == 0xcb) {

Hoist and name this comparision (e.g. bool signal = ...).

> -static int wfc64(const S390Vector *v1, const S390Vector *v2,
> -                 CPUS390XState *env, bool signal, uintptr_t retaddr)
> -{
> -    /* only the zero-indexed elements are compared */
> -    const float64 a = s390_vec_read_element64(v1, 0);
> -    const float64 b = s390_vec_read_element64(v2, 0);
> -    uint8_t vxc, vec_exc = 0;
> -    int cmp;
> -
> -    if (signal) {
> -        cmp = float64_compare(a, b, &env->fpu_status);
> -    } else {
> -        cmp = float64_compare_quiet(a, b, &env->fpu_status);
> -    }
> -    vxc = check_ieee_exc(env, 0, false, &vec_exc);
> -    handle_ieee_exc(env, vxc, vec_exc, retaddr);
> -
> -    return float_comp_to_cc(env, cmp);
> +#define DEF_WFC(BITS)                                                        
>   \
> +static int wfc##BITS(const S390Vector *v1, const S390Vector *v2,             
>   \
> +                     CPUS390XState *env, bool signal, uintptr_t retaddr)     
>   \
> +{                                                                            
>   \
> +    /* only the zero-indexed elements are compared */                        
>   \
> +    const float##BITS a = s390_vec_read_float##BITS(v1, 0);                  
>   \
> +    const float##BITS b = s390_vec_read_float##BITS(v2, 0);                  
>   \
> +    uint8_t vxc, vec_exc = 0;                                                
>   \
> +    int cmp;                                                                 
>   \
> +                                                                             
>   \
> +    if (signal) {                                                            
>   \
> +        cmp = float##BITS##_compare(a, b, &env->fpu_status);                 
>   \
> +    } else {                                                                 
>   \
> +        cmp = float##BITS##_compare_quiet(a, b, &env->fpu_status);           
>   \
> +    }                                                                        
>   \
> +    vxc = check_ieee_exc(env, 0, false, &vec_exc);                           
>   \
> +    handle_ieee_exc(env, vxc, vec_exc, retaddr);                             
>   \
> +                                                                             
>   \
> +    return float_comp_to_cc(env, cmp);                                       
>   \
>  }
> +DEF_WFC(32)
> +DEF_WFC(64)
> +DEF_WFC(128)

So, same issue here vs debugging.

If you keep this macroized, I don't see the value in two levels of macros...

> +#define DEF_GVEC_WFC(BITS)                                                   
>   \
> +void HELPER(gvec_wfc##BITS)(const void *v1, const void *v2, CPUS390XState 
> *env,\
> +                            uint32_t desc)                                   
>   \
> +{                                                                            
>   \
> +    env->cc_op = wfc##BITS(v1, v2, env, false, GETPC());                     
>   \
>  }
> +DEF_GVEC_WFC(32)
> +DEF_GVEC_WFC(64)
> +DEF_GVEC_WFC(128)
>  
> +#define DEF_GVEC_WFK(BITS)                                                   
>   \
> +void HELPER(gvec_wfk##BITS)(const void *v1, const void *v2, CPUS390XState 
> *env,\
> +                            uint32_t desc)                                   
>   \
> +{                                                                            
>   \
> +    env->cc_op = wfc##BITS(v1, v2, env, true, GETPC());                      
>   \
>  }
> +DEF_GVEC_WFK(32)
> +DEF_GVEC_WFK(64)
> +DEF_GVEC_WFK(128)

These could be folded in to the first macro via parameters.


r~