fpu/softfloat-specialize.h | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com>
Wrong type of NaN was generated for IEEE754-2008 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 | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 16c0bcb..56256be 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,qnan)
+ * 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,qnan)
+ * case sets InvalidOp and returns the default NaN
+ */
+ 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 Mon, 18 Mar 2019 at 18:26, Mateja Marjanovic <mateja.marjanovic@rt-rk.com> wrote: > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > Wrong type of NaN was generated for IEEE754-2008 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> > @@ -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,qnan) > + * case sets InvalidOp and returns the default NaN > + */ > + if (infzero) { > + float_raise(float_flag_invalid, status); > + return 2; The comment says we return the default NaN, but the code says we return the input NaN (ie the input value 'c'). Which is correct? > + } > /* Prefer sNaN over qNaN, in the c, a, b order. */ > if (is_snan(c_cls)) { > return 2; thanks -- PMM
On 18.3.19. 19:30, Peter Maydell wrote: > On Mon, 18 Mar 2019 at 18:26, Mateja Marjanovic > <mateja.marjanovic@rt-rk.com> wrote: >> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> >> >> Wrong type of NaN was generated for IEEE754-2008 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> >> @@ -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,qnan) >> + * case sets InvalidOp and returns the default NaN >> + */ >> + if (infzero) { >> + float_raise(float_flag_invalid, status); >> + return 2; > The comment says we return the default NaN, but the code says > we return the input NaN (ie the input value 'c'). Which is correct? It should be the input value 'c', I wrote that by mistake. It will be corrected in v3. >> + } >> /* Prefer sNaN over qNaN, in the c, a, b order. */ >> if (is_snan(c_cls)) { >> return 2; > thanks > -- PMM Thanks for noticing. Regards, Mateja
Hi, Mateja, > From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > Subject: [PATCH] target/mips: Fix minor bug in FPU > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > Wrong type of NaN was generated for IEEE754-2008 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 | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h > index 16c0bcb..56256be 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,qnan) "IEEE754-1985" should be "IEEE 754-1985". "(inf,zero,qnan)" should be "(inf,zero,nan)" (the "c" can be both kinds of NaN at the moment of execution of this block, as it is visible from the code surrounding invocations of pickNaNMulAdd()). > + * 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,qnan) A similar comment applies, like for the block above. > + * case sets InvalidOp and returns the default NaN This line is incorrect, and, instead of "the default NaN", there should be "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 Reference to the justification in documentation in the commit message would be useful and nice. Thanks, Aleksandar
On 18.3.19. 19:55, Aleksandar Markovic wrote: > Hi, Mateja, > >> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >> Subject: [PATCH] target/mips: Fix minor bug in FPU >> >> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> >> >> Wrong type of NaN was generated for IEEE754-2008 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 | 24 ++++++++++++++++-------- >> 1 file changed, 16 insertions(+), 8 deletions(-) >> >> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h >> index 16c0bcb..56256be 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,qnan) > "IEEE754-1985" should be "IEEE 754-1985". > > "(inf,zero,qnan)" should be "(inf,zero,nan)" (the "c" can be both kinds of NaN at > the moment of execution of this block, as it is visible from the code surrounding > invocations of pickNaNMulAdd()). It was a copy-paste error. In the previous version I thought the if-else statement reffers to the snan and qnan. Later I found out it actually checks if it is IEEE754-1985 or IEEE754-2008. It will be corrected as soon as possible. > >> + * 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,qnan) > A similar comment applies, like for the block above. Same goes for this. > >> + * case sets InvalidOp and returns the default NaN > This line is incorrect, and, instead of "the default NaN", there should be "c". Yes, I noticed that, Peter already wrote me, but thanks anyways. > >> + */ >> + 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 > Reference to the justification in documentation in the commit message would be > useful and nice. I agree, it will be added. > > Thanks, > Aleksandar Thanks, Mateja
> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > Subject: [PATCH] target/mips: Fix minor bug in FPU > > From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > > Wrong type of NaN was generated for IEEE754-2008 by maddf and > msubf insturctions when the arguments were inf, zero, nan or > zero, inf, nan respectively. > "insturctions" -> "instructions". The word "respectively" is here wrong and spurious. The meaning that you convey with the current wording is that the "inf, zero, nan" case is related to "maddf" (only), and the "zero, inf, nan" to "msubf". Please use full instruction names with possible Backus-Naur shorthands, like "MADDF.<D|S>", "MSUBF.<D|S>". Thanks, Aleksandar
On 18.3.19. 20:08, Aleksandar Markovic wrote: >> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >> Subject: [PATCH] target/mips: Fix minor bug in FPU >> >> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> >> >> Wrong type of NaN was generated for IEEE754-2008 by maddf and >> msubf insturctions when the arguments were inf, zero, nan or >> zero, inf, nan respectively. >> > "insturctions" -> "instructions". Typo, I will correct that. > > The word "respectively" is here wrong and spurious. > > The meaning that you convey with the current wording is that > the "inf, zero, nan" case is related to "maddf" (only), and the > "zero, inf, nan" to "msubf". Could you tell me what do you suggest? Does "Wrong type of NaN was generated for IEEE754-2008 by maddf and msubf insturctions when the arguments were (inf, zero, nan) or (zero, inf, nan)." seem ok? > > Please use full instruction names with possible Backus-Naur > shorthands, like "MADDF.<D|S>", "MSUBF.<D|S>". Ok, I will use that from now on. > > Thanks, > Aleksandar Thanks, Mateja
> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > Subject: Re: [PATCH] target/mips: Fix minor bug in FPU > > > On 18.3.19. 20:08, Aleksandar Markovic wrote: > >> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > >> Subject: [PATCH] target/mips: Fix minor bug in FPU > >> > >> From: Mateja Marjanovic <Mateja.Marjanovic@rt-rk.com> > >> > >> Wrong type of NaN was generated for IEEE754-2008 by maddf and > >> msubf insturctions when the arguments were inf, zero, nan or > >> zero, inf, nan respectively. > >> > > "insturctions" -> "instructions". > Typo, I will correct that. > > > > The word "respectively" is here wrong and spurious. > > > > The meaning that you convey with the current wording is that > > the "inf, zero, nan" case is related to "maddf" (only), and the > > "zero, inf, nan" to "msubf". > Could you tell me what do you suggest? > > Does "Wrong type of NaN was generated for IEEE754-2008 by maddf and msubf > > insturctions when the arguments were (inf, zero, nan) or > > (zero, inf, nan)." seem ok? I'm afraid it doesn't, it contains several errors that I already objected to. I suggest that you read the comments thoroughly. Aleksandar > > > > > Please use full instruction names with possible Backus-Naur > > shorthands, like "MADDF.<D|S>", "MSUBF.<D|S>". > Ok, I will use that from now on. > > > > Thanks, > > Aleksandar > Thanks, > Mateja >
> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> > Subject: [PATCH] target/mips: Fix minor bug in FPU "v2" is missing in the subject line, and now v1 and v2 are incorrectly linked on mailing list website. An isolated patch is perhaps better sent without cover letter - especially since your cover letter doesn't add any information that is not it the commit message (they are actually identical). What is missing is also the history of changes. Please read: https://wiki.qemu.org/Contribute/SubmitAPatch and https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297 carefully. Thanks, Aleksandar
On 18.3.19. 20:42, Aleksandar Markovic wrote: >> From: Mateja Marjanovic <mateja.marjanovic@rt-rk.com> >> Subject: [PATCH] target/mips: Fix minor bug in FPU > "v2" is missing in the subject line, and now v1 and v2 are incorrectly > linked on mailing list website. I accidentally forgot to add v2 in the subject line. > > An isolated patch is perhaps better sent without cover letter - > especially since your cover letter doesn't add any information > that is not it the commit message (they are actually identical). I didn't know it could be without the cover letter, I won't make that mistake again. > > What is missing is also the history of changes. > > Please read: > > https://wiki.qemu.org/Contribute/SubmitAPatch > > and > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297 > > carefully. I will read them, thanks. > > Thanks, > Aleksandar Thanks, Mateja
© 2016 - 2024 Red Hat, Inc.