| Message ID | 1553008916-15274-2-git-send-email-mateja.marjanovic@rt-rk.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] target/mips: Fix minor bug in FPU | expand |
On Tue, 19 Mar 2019 at 15:22, Mateja Marjanovic <mateja.marjanovic@rt-rk.com> wrote: > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and > MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or > (zero, inf, nan). > These instructions were tested and the results match with the results > of the machine that has a MIPS64 r6 cpu. > > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > --- > fpu/softfloat-specialize.h | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Mateja Marjanovic <mateja.marjanovic@rt-rk.com> writes: > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and > MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or > (zero, inf, nan). > These instructions were tested and the results match with the results > of the machine that has a MIPS64 r6 cpu. > > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> Queued to fpu/next, thanks. -- Alex Bennée
Aleksandar Markovic <amarkovic@wavecomp.com> writes: >> From: Alex Bennée <alex.bennee@linaro.org> >> Subject: Re: [PATCH v3] target/mips: Fix minor bug in FPU >> >> Queued to fpu/next, thanks. > > Alex, does this mean for 4.0 or 4.1? It's a bug fix so it will go in 4.0 I think. -- Alex Bennée
> >> From: Alex Bennée <alex.bennee@linaro.org> > >> Subject: Re: [PATCH v3] target/mips: Fix minor bug in FPU > >> > >> Queued to fpu/next, thanks. > > > > Alex, does this mean for 4.0 or 4.1? > > It's a bug fix so it will go in 4.0 I think. Right, Alex, I also think it should be included in 4.0. Please include this patch in such queue if possible. The "next" in "fpu/next" scared me, I thought it implies "4.1". Sincerely, Aleksandar
> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > Subject: [PATCH v3] target/mips: Fix minor bug in FPU > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and > MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or > (zero, inf, nan). > These instructions were tested and the results match with the results > of the machine that has a MIPS64 r6 cpu. > > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > --- Mateja, The commit message is missing information on the reference source of information, that you placed in the cover letter. In my opinion, this is an important peace of information for anybody seeing and examining this patch in future, so please submit a new version of the patch as an isolated patch and updated commit message. Thanks in advance, Aleksandar > fpu/softfloat-specialize.h | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h > index 16c0bcb..7b88957 100644 > --- a/fpu/softfloat-specialize.h > +++ b/fpu/softfloat-specialize.h > @@ -495,15 +495,15 @@ static int pickNaNMulAdd(FloatClass a_cls, FloatClass b_cls, FloatClass > c_cls, > return 1; > } > #elif defined(TARGET_MIPS) > - /* For MIPS, the (inf,zero,qnan) case sets InvalidOp and returns > - * the default NaN > - */ > - if (infzero) { > - float_raise(float_flag_invalid, status); > - return 3; > - } > - > if (snan_bit_is_one(status)) { > + /* > + * For MIPS systems that conform to IEEE754-1985, the (inf,zero,nan) > + * case sets InvalidOp and returns the default NaN > + */ > + if (infzero) { > + float_raise(float_flag_invalid, status); > + return 3; > + } > /* Prefer sNaN over qNaN, in the a, b, c order. */ > if (is_snan(a_cls)) { > return 0; > @@ -519,6 +519,14 @@ static int pickNaNMulAdd(FloatClass a_cls, FloatClass b_cls, FloatClass > c_cls, > return 2; > } > } else { > + /* > + * For MIPS systems that conform to IEEE754-2008, the (inf,zero,nan) > + * case sets InvalidOp and returns the input value 'c' > + */ > + if (infzero) { > + float_raise(float_flag_invalid, status); > + return 2; > + } > /* Prefer sNaN over qNaN, in the c, a, b order. */ > if (is_snan(c_cls)) { > return 2; > -- > 2.7.4 >
On Tue, 19 Mar 2019 at 19:21, Aleksandar Markovic <amarkovic@wavecomp.com> wrote: > > > From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > > Subject: [PATCH v3] target/mips: Fix minor bug in FPU > > > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and > > MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or > > (zero, inf, nan). > > These instructions were tested and the results match with the results > > of the machine that has a MIPS64 r6 cpu. > > > > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > > --- > > Mateja, > > The commit message is missing information on the reference source > of information, that you placed in the cover letter. In my opinion, this is > an important peace of information for anybody seeing and examining > this patch in future, so please submit a new version of the patch as an > isolated patch and updated commit message. If Alex is willing to edit the commit message in his tree that would save Mateja from having to do another respin. thanks -- PMM
> > From: Peter Maydell <peter.maydell@linaro.org> > Subject: Re: [PATCH v3] target/mips: Fix minor bug in FPU > > On Tue, 19 Mar 2019 at 19:21, Aleksandar Markovic > <amarkovic@wavecomp.com> wrote: > > > > > From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > > > Subject: [PATCH v3] target/mips: Fix minor bug in FPU > > > > > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > > > > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and > > > MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or > > > (zero, inf, nan). > > > These instructions were tested and the results match with the results > > > of the machine that has a MIPS64 r6 cpu. > > > > > > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > > > --- > > > > Mateja, > > > > The commit message is missing information on the reference source > > of information, that you placed in the cover letter. In my opinion, this is > > an important peace of information for anybody seeing and examining > > this patch in future, so please submit a new version of the patch as an > > isolated patch and updated commit message. > > If Alex is willing to edit the commit message in his tree > that would save Mateja from having to do another respin. > Sure, that would be a good option. Whoever (Alex or Mateja) modifies the patch, the commit message should please read: (I did some rearranging and editing) Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and MSUBF.<D|S> instructions when the arguments were (Inf, Zero, NaN) or (Zero, Inf, NaN). The if-else statement establishes if the system conforms to IEEE 754-1985 or IEEE 754-2008, and defines different behaviors depending on that. In case of IEEE 754-2008, in mentioned cases of inputs, <MADDF|MSUBF>.<D|S> returns the input value 'c' [2] (page 53) and raises floating point exception 'Invalid Operation' [1] (pages 349, 350). These scenarios were tested and the results in QEMU emulation match the results obtained on the machine that has a MIPS64R6 CPU. [1] MIPS Architecture for Programmers Volume II-a: The MIPS64 Instruction Set Reference Manual, Revision 6.06 [2] MIPS Architecture for Programmers Volume IV-j: The MIPS64 SIMD Architecture Module, Revision 1.12 Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > thanks > -- PMM
Aleksandar Markovic <amarkovic@wavecomp.com> writes: >> >> From: Peter Maydell <peter.maydell@linaro.org> >> Subject: Re: [PATCH v3] target/mips: Fix minor bug in FPU >> >> On Tue, 19 Mar 2019 at 19:21, Aleksandar Markovic >> <amarkovic@wavecomp.com> wrote: >> > >> > > From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >> > > Subject: [PATCH v3] target/mips: Fix minor bug in FPU >> > > >> > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> >> > > >> > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and >> > > MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or >> > > (zero, inf, nan). >> > > These instructions were tested and the results match with the results >> > > of the machine that has a MIPS64 r6 cpu. >> > > >> > > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >> > > --- >> > >> > Mateja, >> > >> > The commit message is missing information on the reference source >> > of information, that you placed in the cover letter. In my opinion, this is >> > an important peace of information for anybody seeing and examining >> > this patch in future, so please submit a new version of the patch as an >> > isolated patch and updated commit message. >> >> If Alex is willing to edit the commit message in his tree >> that would save Mateja from having to do another respin. >> > > Sure, that would be a good option. Whoever (Alex or Mateja) modifies > the patch, the commit message should please read: (I did some > rearranging and editing) > > > Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and > MSUBF.<D|S> instructions when the arguments were (Inf, Zero, NaN) or > (Zero, Inf, NaN). > > The if-else statement establishes if the system conforms to IEEE > 754-1985 or IEEE 754-2008, and defines different behaviors depending > on that. In case of IEEE 754-2008, in mentioned cases of inputs, > <MADDF|MSUBF>.<D|S> returns the input value 'c' [2] (page 53) and > raises floating point exception 'Invalid Operation' [1] (pages 349, > 350). > > These scenarios were tested and the results in QEMU emulation match > the results obtained on the machine that has a MIPS64R6 CPU. > > [1] MIPS Architecture for Programmers Volume II-a: The MIPS64 > Instruction Set Reference Manual, Revision 6.06 > [2] MIPS Architecture for Programmers Volume IV-j: The MIPS64 > SIMD Architecture Module, Revision 1.12 > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> I've updated the commit message, thanks. -- Alex Bennée
On 19.3.19. 16:27, Peter Maydell wrote: > On Tue, 19 Mar 2019 at 15:22, Mateja Marjanovic > <mateja.marjanovic@rt-rk.com> wrote: >> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> >> >> Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and >> MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or >> (zero, inf, nan). >> These instructions were tested and the results match with the results >> of the machine that has a MIPS64 r6 cpu. >> >> Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >> --- >> fpu/softfloat-specialize.h | 24 ++++++++++++++++-------- >> 1 file changed, 16 insertions(+), 8 deletions(-) >> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Thank you for reviewing. > thanks > -- PMM Regards, Mateja
On 19.3.19. 23:02, Alex Bennée wrote: > Aleksandar Markovic <amarkovic@wavecomp.com> writes: > >>> From: Peter Maydell <peter.maydell@linaro.org> >>> Subject: Re: [PATCH v3] target/mips: Fix minor bug in FPU >>> >>> On Tue, 19 Mar 2019 at 19:21, Aleksandar Markovic >>> <amarkovic@wavecomp.com> wrote: >>>>> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >>>>> Subject: [PATCH v3] target/mips: Fix minor bug in FPU >>>>> >>>>> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> >>>>> >>>>> Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and >>>>> MSUBF.<D|S> instructions when the arguments were (inf, zero, nan) or >>>>> (zero, inf, nan). >>>>> These instructions were tested and the results match with the results >>>>> of the machine that has a MIPS64 r6 cpu. >>>>> >>>>> Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >>>>> --- >>>> Mateja, >>>> >>>> The commit message is missing information on the reference source >>>> of information, that you placed in the cover letter. In my opinion, this is >>>> an important peace of information for anybody seeing and examining >>>> this patch in future, so please submit a new version of the patch as an >>>> isolated patch and updated commit message. >>> If Alex is willing to edit the commit message in his tree >>> that would save Mateja from having to do another respin. >>> >> Sure, that would be a good option. Whoever (Alex or Mateja) modifies >> the patch, the commit message should please read: (I did some >> rearranging and editing) >> >> >> Wrong type of NaN was generated for IEEE 754-2008 by MADDF.<D|S> and >> MSUBF.<D|S> instructions when the arguments were (Inf, Zero, NaN) or >> (Zero, Inf, NaN). >> >> The if-else statement establishes if the system conforms to IEEE >> 754-1985 or IEEE 754-2008, and defines different behaviors depending >> on that. In case of IEEE 754-2008, in mentioned cases of inputs, >> <MADDF|MSUBF>.<D|S> returns the input value 'c' [2] (page 53) and >> raises floating point exception 'Invalid Operation' [1] (pages 349, >> 350). >> >> These scenarios were tested and the results in QEMU emulation match >> the results obtained on the machine that has a MIPS64R6 CPU. >> >> [1] MIPS Architecture for Programmers Volume II-a: The MIPS64 >> Instruction Set Reference Manual, Revision 6.06 >> [2] MIPS Architecture for Programmers Volume IV-j: The MIPS64 >> SIMD Architecture Module, Revision 1.12 >> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > I've updated the commit message, thanks. I really appreciate this. > -- > Alex Bennée Regards, Mateja
diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h index 16c0bcb..7b88957 100644 --- a/fpu/softfloat-specialize.h +++ b/fpu/softfloat-specialize.h @@ -495,15 +495,15 @@ static int pickNaNMulAdd(FloatClass a_cls, FloatClass b_cls, FloatClass c_cls, return 1; } #elif defined(TARGET_MIPS) - /* For MIPS, the (inf,zero,qnan) case sets InvalidOp and returns - * the default NaN - */ - if (infzero) { - float_raise(float_flag_invalid, status); - return 3; - } - if (snan_bit_is_one(status)) { + /* + * For MIPS systems that conform to IEEE754-1985, the (inf,zero,nan) + * case sets InvalidOp and returns the default NaN + */ + if (infzero) { + float_raise(float_flag_invalid, status); + return 3; + } /* Prefer sNaN over qNaN, in the a, b, c order. */ if (is_snan(a_cls)) { return 0; @@ -519,6 +519,14 @@ static int pickNaNMulAdd(FloatClass a_cls, FloatClass b_cls, FloatClass c_cls, return 2; } } else { + /* + * For MIPS systems that conform to IEEE754-2008, the (inf,zero,nan) + * case sets InvalidOp and returns the input value 'c' + */ + if (infzero) { + float_raise(float_flag_invalid, status); + return 2; + } /* Prefer sNaN over qNaN, in the c, a, b order. */ if (is_snan(c_cls)) { return 2;