Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] target-mips: Implement IEEE 754-2008 functionality for R6 and MSA instructions

[Aleksandar Markovic]

Hi, Richard, what would you think about this approach:

Functionality of <ABS|NEG>.<S|D> and <CVT|FLOOR|CEIL|TRUNC|ROUND>.<L|W>.<S|D>
instructions is dependent on flags ABS2008 and NAN2008 in FCR31. There are
MIPS architectures (for example mips32r5) that allow implementations
with different values of these flags. So, in order to detect the desired
behavior in translate-time, insn_flags field can't be used - and, therefore,
it makes sense to add two new members to the MIPS's DisasContext:

typedef struct DisasContext {
    . . .
    bool nan2008;
    bool abs2008;
} DisasContext;

Their initialization could be in gen_intermediate_code_internal():

    ctx.nan2008 = (env->active_fpu.fcr31 >> FCR31_NAN2008) & 1;
    ctx.abs2008 = (env->active_fpu.fcr31 >> FCR31_ABS2008) & 1;

Now, ABS.D (and all <ABS|NEG>.<S|D>) handling might look like this:

    case OPC_ABS_D:
        check_cp1_registers(ctx, fs | fd);
        {
            TCGv_i64 fp0 = tcg_temp_new_i64();

            gen_load_fpr64(ctx, fp0, fs);
            if (ctx->abs2008) {
                tcg_gen_andi_i64(fp0, fp0, 0x7fffffffffffffffULL);
            } else {
                gen_helper_float_abs_d(fp0, fp0);
            }
            gen_store_fpr64(ctx, fp0, fd);
            tcg_temp_free_i64(fp0);
        }
        opn = "abs.d";
        break;

Here, 2008-style ABS.D is implemented inline, without a helper, and
gen_helper_float_abs_d() is an old pre-2008 helper that would be intact
(the same as it is currently) with this change.

On the other hand, CVT.L.D (and all <CVT|FLOOR|CEIL|TRUNC|ROUND>.<L|W>.<S|D>)
handling would take this form:

    case OPC_CVT_L_D:
        check_cp1_64bitmode(ctx);
        {
            TCGv_i64 fp0 = tcg_temp_new_i64();

            gen_load_fpr64(ctx, fp0, fs);
            if (ctx->nan2008) {
                gen_helper_float_cvt_2008_l_d(fp0, cpu_env, fp0);
            } else {
                gen_helper_float_cvt_l_d(fp0, cpu_env, fp0);
            }
            gen_store_fpr64(ctx, fp0, fd);
            tcg_temp_free_i64(fp0);
        }
        opn = "cvt.l.d";
        break;

Function helper_float_cvt_2008_l_d() is a new, only-2008-style helper for
CVT.L.D and would look like this:

uint64_t helper_float_cvt_2008_l_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 = DBL_TO_INT64_OVERFLOW(fdt0)
    }
    update_fcr31(env, GETPC());
    return dt2;
}

(macro DBL_TO_INT64_OVERFLOW(x) would be defined this way:

#define DBL_TO_INT64_OVERFLOW(x)                                             \
    float64_is_any_nan(x) ? 0 : (float64_is_neg(x) ? INT64_MIN : INT64_MAX);

to avoid awkward repeating "if" statements in multiple headers)

gen_helper_float_cvt_l_d() and all old style helpers for instructions
<CVT|FLOOR|CEIL|TRUNC|ROUND>.<L|W>.<S|D> would remain the same.

Please let me know about your opinion. I greatly appreciate your kind
consideration of this matter. I am looking forward to hearing from you.

Yours,
Aleksandar
________________________________________
From: address@hidden address@hidden on behalf of Richard Henderson 
address@hidden
Sent: Monday, March 28, 2016 2:49 PM
To: Aleksandar Markovic; address@hidden
Cc: address@hidden; address@hidden; address@hidden; address@hidden; 
address@hidden; Petar Jovanovic; address@hidden; address@hidden; Miodrag Dinic; 
address@hidden; address@hidden; address@hidden; address@hidden; address@hidden; 
Leon Alrae; address@hidden; address@hidden; address@hidden
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~