Re: [Qemu-devel] [PATCH 3/9] Add risu_reginfo_ppc.c file

From: Peter Maydell <peter.maydell@linaro.org>
To: G 3 <programmingkidx@gmail.com>
Cc: "qemu-devel@nongnu.org qemu-devel" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 3/9] Add risu_reginfo_ppc.c file
Date: Tue, 25 Apr 2017 09:17:46 +0100	[thread overview]
Message-ID: <CAFEAcA_PUdFS-gPncf9S6ZGyKGdotang=sM1EkE42M7OKTW16w@mail.gmail.com> (raw)
In-Reply-To: <BD9E8ECB-EB56-45A9-89A4-C1506319A7BD@gmail.com>

On 25 April 2017 at 04:19, G 3 <programmingkidx@gmail.com> wrote:
> Add the risu_reginfo_ppc.c file. It handles operations involving the reginfo
> structure.

> +    /* check each floating point register */
> +    for (i = 0; i < NUMBER_OF_FPR; i++) {
> +        if (r1->fpr[i] != r2->fpr[i]) {
> +            if (!(isnan(r1->fpr[i]) && isnan(r2->fpr[i]))) {
> +                if ( fabs(r1->fpr[i] - r2->fpr[i]) < 0.000001) {
> +                    debug_print("float point register %d mismatch
> detected\n", i);
> +                    return 0;
> +                }

This is definitely wrong. Risu is supposed to check for exact
binary correctness, so you simply want to compare the binary
values of the FP regs, not check whether they're vaguely
close to the right answer.

> +/*
> + * Shows the classification of a floating point value.
> + * Input: floating point value
> + * Output: string description
> + */
> +const char *show_classification(double x) {
> +    switch(fpclassify(x)) {
> +        case FP_INFINITE:  return "Inf";
> +        case FP_NAN:       return "NaN";
> +        case FP_NORMAL:    return "normal";
> +        case FP_SUBNORMAL: return "subnormal";
> +        case FP_ZERO:      return "zero";
> +        default:           return "unknown";
> +    }
> +}

None of the other backends do this. If we want to do it
(and I'm not convinced it's worth the effort) we should
do it consistently everywhere.

thanks
-- PMM