Aleksandar Markovic <amarko...@wavecomp.com> writes:
>> From: Alex Bennée <alex.ben...@linaro.org>
>> Subject: Re: [PATCH] target/mips: Fix minor bug in FPU
>>
>> Mateja Marjanovic <mateja.marjano...@rt-rk.com> writes:
>>
>> > From: Mateja Marjanovic <mateja.marjano...@rt-rk.com>
>> >
>> > Wrong type of NaN was generated by maddf and msubf insturctions
>> > when the arguments were inf, zero, nan or zero, inf, nan
>> > respectively.
>> >
>> > Signed-off-by: Mateja Marjanovic <mateja.marjano...@rt-rk.com>
>> > ---
>> > fpu/softfloat-specialize.h | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
>> > index 16c0bcb..647bfbc 100644
>> > --- a/fpu/softfloat-specialize.h
>> > +++ b/fpu/softfloat-specialize.h
>> > @@ -500,7 +500,7 @@ static int pickNaNMulAdd(FloatClass a_cls, FloatClass
>> > b_cls, FloatClass c_cls,
>> > */
>> > if (infzero) {
>> > float_raise(float_flag_invalid, status);
>> > - return 3;
>> > + return 2;
>>
>> Hi,
>>
>> This changes the behaviour documented above which says:
>>
>> /* For MIPS, the (inf,zero,qnan) case sets InvalidOp and returns
>> * the default NaN
>> */
>>
>> So if the behaviour is incorrect please update the comment as well.
>> Bonus points for a reference to the canonical reference document that
>> describes this.
>>
>
> Alex,
>
> I tested this case (0*inf+nan fused multiply-add/multiply-subtract) on a MIPS
> hardware system, and this patch is right - after the patch QEMU and hardware
> (MIPS64R6 board) behaviors match. The comment you mentioned probably
> just reflects the code, it looks not to be the reference source of
> information.
> If the patch is accepted, that comments must be changed in the same
> patch.
I'm perfectly happy to take your word the fix is fine for MIPS - as you
say this is part of the rich tapestry of IMPDEF variations on NaN
handling ;-)
I did have a brief browse through the MIPS Architecture for Programmers
(Document Number: MD00083) but couldn't find a decent line about the NaN
propagation but if it's somewhere else it would be worth mentioning in
the comment.
>
> This is a MIPS-only-specific change (under "#if defined (TARGET_MIPS)"), and,
> from your point of view, could this be included in MIPS pull request (if the
> validity
> of the patch is confirmed)?
I'd be happy for you to included it (with the matching comment change)
in your next PR:
Acked-by: Alex Bennée <alex.ben...@linaro.org>
>
> Sincerely,
> Aleksandar
--
Alex Bennée
