Re: [Qemu-devel] [PATCH] target/mips: Fix minor bug in FPU

[Alex Bennée]

Aleksandar Markovic <address@hidden> writes:

>> From: Peter Maydell <address@hidden>
>> Subject: Re: [PATCH] target/mips: Fix minor bug in FPU
>>
>> On Mon, 11 Mar 2019 at 11:52, Aleksandar Markovic
>> <address@hidden> wrote:
>> >
>> > > From: Mateja Marjanovic <address@hidden>
>> > > Subject: [PATCH] target/mips: Fix minor bug in FPU
>> > >
>> > > Wrong type of NaN was generated by maddf and msubf insturctions
>> > > when the arguments were inf, zero, nan or zero, inf, nan
>> > > respectively.
>> >
>> > I did the applicable tests on both pre-NaN2008 and NaN2008 MIPS hardware,
>> > and compared results with QEMu emulations. The underlying reason for this
>> > patch is correct, but, as Alex also pointed out, it needs some 
>> > improvements.
>> > However, the softfreeze being so close, I am going to amend the patch while
>> > creating the pull request. No respin needed. All in all:
>>
>> Since this is a bug fix, there is no requirement that it goes in
>> before softfreeze, FWIW -- pretty much any bug fix is OK for
>> rc1, and a focused bugfix like this one that doesn't affect
>> other guest architectures would be ok in rc2 as well.
>>
>
> Thanks, Peter, for the clarification!
>
> In this case, I think, the most natural would be to wait for the
> submitter to submit the v2.
>
> The reference to the documentation that Alex rightfully asked for
> is this:
>
> "MIPS Architecture for Programmers Volume IV-j: The MIPS32
> SIMD Architecture Module", Revision 1.12, February 3, 2016
>
> The key sentence is on page 53:
>
>    "If the destination format is floating-point, all NaN propagating
>    operations with one NaN operand produce a NaN with the
>    payload of the input NaN."

Hmm doesn't that fail for:

  Inf * Zero + !NaN?

Should the check be:

    if (infzero) {
        float_raise(float_flag_invalid, status);
        if (is_nan(c_cls)) {
            return 2;
        }
        return 3;
    }

?

If either a or b is a NaN we fall through to the NaN selection rules
bellow preferring SNaN over QNaN.

>
> There are other details in surrounding paragraphs. The document
> is for SIMD (MSA) ASE, but both the SIMD and FPU NaN2008
> floating point instruction should follow the same rules. I could't
> find a separate document on FPU instructions.
>
> The problem with the current implementation of 0*inf+nan is that
> it returns the default NaN (all zeroes in the payload segment),
> whereas the documentation says the payload should be from
> the input NaN. The behavior specified in the documentation is
> confirmed also with tests on hardware.
>
> Sincerely,
> Aleksandar


--
Alex Bennée