| Message ID | 1551978650-23207-2-git-send-email-mateja.marjanovic@rt-rk.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/mips: Fix minor bug in FPU | expand |
Mateja Marjanovic <mateja.marjanovic@rt-rk.com> writes: > From: Mateja Marjanovic <Mateja.Marjanovic@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.marjanovic@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 Bennée
Aleksandar Markovic <amarkovic@wavecomp.com> writes: >> From: Alex Bennée <alex.bennee@linaro.org> >> Subject: Re: [PATCH] target/mips: Fix minor bug in FPU >> >> Mateja Marjanovic <mateja.marjanovic@rt-rk.com> writes: >> >> > From: Mateja Marjanovic <Mateja.Marjanovic@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.marjanovic@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.bennee@linaro.org> > > Sincerely, > Aleksandar -- Alex Bennée
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; } if (snan_bit_is_one(status)) {