1. Patchew
  2. QEMU
  3. View series

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

Mateja Marjanovic posted 1 patch 5 years, 1 month ago
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1553008916-15274-2-git-send-email-mateja.marjanovic@rt-rk.com
Maintainers: Aleksandar Markovic <amarkovic@wavecomp.com>, Peter Maydell <peter.maydell@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <arikalo@wavecomp.com>, "Alex Bennée" <alex.bennee@linaro.org>
fpu/softfloat-specialize.h | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Mateja Marjanovic 5 years, 1 month ago 
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(-)

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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Alex Bennée 5 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Aleksandar Markovic 5 years, 1 month ago 
> 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?

Aleksandar
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Alex Bennée 5 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Aleksandar Markovic 5 years, 1 month ago 
> >> 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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Aleksandar Markovic 5 years, 1 month ago 
> 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
>
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Peter Maydell 5 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Aleksandar Markovic 5 years, 1 month ago 
> 
> 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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Alex Bennée 5 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Mateja Marjanovic 5 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Peter Maydell 5 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH v3] target/mips: Fix minor bug in FPU
Posted by Mateja Marjanovic 5 years, 1 month ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.