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

[G 3]

On Apr 25, 2017, at 4:17 AM, Peter Maydell wrote:

> On 25 April 2017 at 04:19, G 3 <address@hidden> 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.

Remove the isnan and fabs lines?


> > +/*
> > + * 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.

It does help with understanding what is different with two floating  
point values. But if you don't want it removing it is fine with me.