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

[Nikunj A 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