> From: Alex Bennée <alex.ben...@linaro.org> > > > Aleksandar Markovic <amarko...@wavecomp.com> writes: > > >> From: Peter Maydell <peter.mayd...@linaro.org> > >> Subject: Re: [PATCH] target/mips: Fix minor bug in FPU > >> > >> On Mon, 11 Mar 2019 at 11:52, Aleksandar Markovic > >> <amarko...@wavecomp.com> wrote: > >> > > >> > > From: Mateja Marjanovic <mateja.marjano...@rt-rk.com> > >> > > 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. >
Alex, I'll doublecheck, but I think "infzero" here is a misnomer. The variable/argument "infzero" actually denotes the cases: - "Inf * Zero +/- NaN" - "Inf * Zero +/- NaN" - "Zero * Inf +/- NaN" - "Zero * Inf +/- NaN" "Inf * Zero +/- !NaN (let's say, normal fp)" is handled somewhere else. Therefore, "infzero" should be rather called "infzeronan". This is from what I remember, but I will reanalyse the relevant softfloat code one more time. Regards, Aleksandar > > > > 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