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

[Nikunj Dadhania]

On 02-Mar-2017 8:07 AM, "Richard Henderson" <address@hidden> wrote:

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?


Only to compute vxisi as stated in the thread
"If the product of x and y is an Infinity and z is an Infinity of the
opposite sign, vxisi_flag is set to 1."

Let me know if I there is an alternative way to achieve this.



> +    /* 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.


Sure.



+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);
  }


Sure.

Nikunj