[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-devel
Subject:    Re: [Qemu-devel] [PATCH 2/2] target-mips: Implement IEEE 754-2008 functionality for R6 and MSA instr
From:       Aleksandar Markovic <Aleksandar.Markovic () imgtec ! com>
Date:       2016-03-30 19:28:33
Message-ID: EF5FA6C3467F85449672C3E735957B859168E22F () BADAG02 ! ba ! imgtec ! org
[Download RAW message or body]

I really appreciate your guidance and help. I will respond shortly with a proposal \
that will address all issues that you brought up. Thanks again for your support and \
time.

Aleksandar
________________________________________
From: qemu-devel-bounces+aleksandar.markovic=imgtec.com@nongnu.org \
[qemu-devel-bounces+aleksandar.markovic=imgtec.com@nongnu.org] on behalf of Richard \
                Henderson [rth@twiddle.net]
Sent: Monday, March 28, 2016 2:49 PM
To: Aleksandar Markovic; qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org; ehabkost@redhat.com; kbastian@mail.uni-paderborn.de; \
mark.cave-ayland@ilande.co.uk; agraf@suse.de; Petar Jovanovic; blauwirbel@gmail.com; \
jcmvbkbc@gmail.com; Miodrag Dinic; qemu-arm@nongnu.org; qemu-ppc@nongnu.org; \
edgar.iglesias@gmail.com; pbonzini@redhat.com; gxt@mprc.pku.edu.cn; Leon Alrae; \
                afaerber@suse.de; aurelien@aurel32.net; proljc@gmail.com
Subject: Re: [Qemu-devel] [PATCH 2/2] target-mips: Implement IEEE 754-2008 \
functionality for R6 and MSA instructions

On 03/25/2016 05:50 AM, Aleksandar Markovic wrote:
> @@ -2621,9 +2621,23 @@ uint64_t helper_float_cvtl_d(CPUMIPSState *env, uint64_t \
> fdt0) uint64_t dt2;
> 
> dt2 = float64_to_int64(fdt0, &env->active_fpu.fp_status);
> -    if (get_float_exception_flags(&env->active_fpu.fp_status)
> -        & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +    if (env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) {
> +        if (get_float_exception_flags(&env->active_fpu.fp_status)
> +                & (float_flag_invalid | float_flag_overflow)) {
> +            if (float64_is_any_nan(fdt0)) {
> +                dt2 = 0;
> +            } else {
> +                if (float64_is_neg(fdt0))
> +                    dt2 = INT64_MIN;
> +                else
> +                    dt2 = INT64_MAX;
> +            }
> +        }
> +    } else {
> +        if (get_float_exception_flags(&env->active_fpu.fp_status)
> +                & (float_flag_invalid | float_flag_overflow)) {
> +            dt2 = FP_TO_INT64_OVERFLOW;
> +        }

Better to swap the tests here, so that you test the exception flags first (and
once).  That is the exceptional condition, the one that will be true least
often.  After that, FCR31_NAN2008 will be tested only when needed.

But also, this pattern is replicated so many times you'd do well to pull this
sequence out to helper functions (one for s, one for d).

> +uint64_t helper_float_abs_d(CPUMIPSState *env, uint64_t fdt0)
> +{
> +    uint64_t fdt1;
> +
> +    if (env->active_fpu.fcr31 & (1 << FCR31_ABS2008)) {
> +        fdt1 = float64_abs(fdt0);
> +    } else {
> +        if (float64_is_neg(fdt0)) {
> +            fdt1 = float64_sub(0, fdt0, &env->active_fpu.fp_status);
> +        } else {
> +            fdt1 = float64_add(0, fdt0, &env->active_fpu.fp_status);
> +        }
> +        update_fcr31(env, GETPC());

Here you're better off using two separate helper functions, and chose the
correct one during translation.  Indeed, since the 2008 version is a simple
bit-flip, you needn't actually have a helper; just expand the sequence inline.


r~


[prev in list] [next in list] [prev in thread] [next in thread]