Re: [Qemu-ppc] [PATCH v1] target/ppc: rewrite f[n]m[add, sub] using float64_muladd

[Richard Henderson]

On 03/02/2017 02:24 AM, Nikunj A Dadhania wrote:
> +static void float64_madd_set_vxisi(CPUPPCState *env, float64 a, float64 b,
> +                                   float64 c, unsigned int flags)
>  {
> +    float64 f = float64_mul(a, b, &env->fp_status);

What is the point of this multiply?
> +    /* a*b = ∞ and c = ∞, find ∞ - ∞ case and set VXISI */
> +    if (float64_is_infinity(f) && float64_is_infinity(c)) {
> +        if ((f ^ c) == 0) {
> +            /* Both negative/positive inifinity and substraction*/
> +            if (flags & MSUB_FLGS) {

I would really prefer you use the float_muladd_* names.

> +uint64_t helper_##op(CPUPPCState *env, uint64_t arg1,                   \
> +                     uint64_t arg2, uint64_t arg3)                      \
> +{                                                                       \
> +    if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) || \
> +                 (float64_is_zero(arg1) && float64_is_infinity(arg2)))) { \
> +        /* Multiplication of zero by infinity */                        \
> +        arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);     \
> +    } else {                                                            \
> +        if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) || \
> +                     float64_is_signaling_nan(arg2, &env->fp_status) || \
> +                     float64_is_signaling_nan(arg3, &env->fp_status))) { \
> +            /* sNaN operation */                                        \
> +            float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);      \
> +        }                                                               \
> +                                                                        \
> +        float64_madd_set_vxisi(env, arg1, arg2, arg3, madd_flags);      \
> +        arg1 = float64_muladd(arg1, arg2, arg3, madd_flags,             \
> +                              &env->fp_status);                         \
> +        float_check_status(env);                                        \

I know this is the layout of the bulk of the ppc target, but it's inefficient. 
Let's do this one correctly, akin to target/tricore:

  result = float64_muladd(args...);
  flags = get_float_exception_flags(&env->fp_status);
  if (flags) {
      if (flags & float_flag_invalid) {
          // examine inputs to see why we return NaN
      }
      float_check_status(env);
  }


r~