Re: [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