Fix NaN handling in softfloat

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

Quoted 8 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

Quoted 2 lines

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Quoted 8 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

Quoted 2 lines

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Quoted 9 lines · Daniel Jacobowitz

Quoted 8 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

Quoted 9 lines

Quoted 2 lines

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

Quoted 20 lines · Aurelien Jarno

Quoted 18 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Quoted 2 lines

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

It's a good idea to fix NaN problems here but in my opinion, it's a bad idea to have target dependant code here. This code should implement IEEE behavior.

Quoted 20 lines · Aurelien Jarno

Quoted 18 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Quoted 2 lines

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

It's a good idea to fix NaN problems here but in my opinion, it's a bad idea to have target dependant code here. This code should implement IEEE behavior. Target specific behavior / deviations from the norm has to be

implemented in target specific code. As targets have to check the presence of a NaN to update the FP flags, it seems that uglyifying this code with target specific hacks is pointless. If the target code do not check the presence of a NaN, that means that it does not implement precise FPU emulation, then there's no need to have specific code to return a precise value (I mean target dependant) from the generic code, imho.

Has Thiemo already said, there is no IEEE behavior. If you look at the IEEE 754 document you will see that it has requirements on what should be supported by an IEEE compliant FPU, but has very few requirements on the implementation.

Quoted 4 lines · Aurelien Jarno

Has Thiemo already said, there is no IEEE behavior. If you look at the IEEE 754 document you will see that it has requirements on what should be supported by an IEEE compliant FPU, but has very few requirements on the implementation.

If folks don't like the target conditionals there, I recommend we just set some low bit to be sure it's a NaN and move on.

The softfloat
implementation is not all that close to matching any one hardware FPU.

Quoted 25 lines · J. Mayer

Quoted 22 lines · Aurelien Jarno

Quoted 9 lines · Daniel Jacobowitz

Quoted 8 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

Quoted 9 lines

Quoted 2 lines

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

Quoted 3 lines

It's a good idea to fix NaN problems here but in my opinion, it's a bad idea to have target dependant code here. This code should implement IEEE behavior. Target specific behavior / deviations from the norm has to be

Has Thiemo already said, there is no IEEE behavior. If you look at the IEEE 754 document you will see that it has requirements on what should be supported by an IEEE compliant FPU, but has very few requirements on the implementation.

Quoted 7 lines

implemented in target specific code. As targets have to check the presence of a NaN to update the FP flags, it seems that uglyifying this code with target specific hacks is pointless. If the target code do not check the presence of a NaN, that means that it does not implement precise FPU emulation, then there's no need to have specific code to return a precise value (I mean target dependant) from the generic code, imho.

I actually know very few CPU that check for NaN in general. They check for sNaN as required by IEEE 754, but rarely for qNaN as their purpose is exactly to be propagated through all FPU operations as a normal FP number would be.

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

Quoted 22 lines · Aurelien Jarno

Quoted 21 lines · J. Mayer

Quoted 20 lines · Aurelien Jarno

Quoted 9 lines · Daniel Jacobowitz

Quoted 8 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

Quoted 8 lines

Quoted 2 lines

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

Quoted 8 lines

Quoted 3 lines

It's a good idea to fix NaN problems here but in my opinion, it's a bad idea to have target dependant code here. This code should implement IEEE behavior. Target specific behavior / deviations from the norm has to be

Has Thiemo already said, there is no IEEE behavior. If you look at the IEEE 754 document you will see that it has requirements on what should be supported by an IEEE compliant FPU, but has very few requirements on the implementation.

OK.

Quoted 11 lines

Quoted 7 lines

implemented in target specific code. As targets have to check the presence of a NaN to update the FP flags, it seems that uglyifying this code with target specific hacks is pointless. If the target code do not check the presence of a NaN, that means that it does not implement precise FPU emulation, then there's no need to have specific code to return a precise value (I mean target dependant) from the generic code, imho.

I actually know very few CPU that check for NaN in general. They check for sNaN as required by IEEE 754, but rarely for qNaN as their purpose is exactly to be propagated through all FPU operations as a normal FP number would be.

CPU do check QNaNs because most of them update a specific flag that can be checked to know there was a NaN seen during FPU operations. I don't know for all FPU, but I can see that the PowerPC gives me 4 bits that give the class of the last FPU result and I guess you have those kind of flags in most implementations.

Quoted 7 lines

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

I don't say you have to return an infinity when the argument is a qNaN. I just say you have to return a qNaN in a generic way. Just return sign | 0x7f800000 | mantissa, which is the more generic form and seems to me to even be OK for sNaNs. It's even needed for some target (not to say

PowerPC) that specify that the result have to be equal to the operand (in the single precision format, of course) in such a case. This is simpler, it ensures that any target could then detect the presence of a NaN, know which one, and can then adjust the value according to its specification if needed.
I then still can'tl see any reason of having target specific code in that area.

Quoted 15 lines · J. Mayer

Quoted 14 lines · Aurelien Jarno

Quoted 13 lines · J. Mayer

Quoted 12 lines · Aurelien Jarno

Quoted 11 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

Quoted 8 lines

Quoted 7 lines

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

Quoted 4 lines

I don't say you have to return an infinity when the argument is a qNaN. I just say you have to return a qNaN in a generic way. Just return sign | 0x7f800000 | mantissa, which is the more generic form and seems to me to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

Quoted 7 lines

PowerPC) that specify that the result have to be equal to the operand (in the single precision format, of course) in such a case. This is simpler, it ensures that any target could then detect the presence of a NaN, know which one, and can then adjust the value according to its specification if needed.
I then still can'tl see any reason of having target specific code in that area.

Ok, let's give an example then. On MIPS let's say you want to convert 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to the right become 0, so you have to generate a new value. As you proposed, let's generate a "generic value" 0x7fc00000 in the softfloat routines. This value has to be converted to 0x7fbfffff in the MIPS target code.

Quoted 16 lines · Aurelien Jarno

Quoted 15 lines · J. Mayer

Quoted 14 lines · Aurelien Jarno

Quoted 13 lines · J. Mayer

Quoted 12 lines · Aurelien Jarno

Quoted 11 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

[...]

Quoted 20 lines

Quoted 11 lines

Quoted 7 lines

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

I don't say you have to return an infinity when the argument is a qNaN. I just say you have to return a qNaN in a generic way. Just return sign | 0x7f800000 | mantissa, which is the more generic form and seems to me to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

Quoted 7 lines

PowerPC) that specify that the result have to be equal to the operand (in the single precision format, of course) in such a case. This is simpler, it ensures that any target could then detect the presence of a NaN, know which one, and can then adjust the value according to its specification if needed.
I then still can'tl see any reason of having target specific code in that area.

Quoted 6 lines

Ok, let's give an example then. On MIPS let's say you want to convert 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to the right become 0, so you have to generate a new value. As you proposed, let's generate a "generic value" 0x7fc00000 in the softfloat routines. This value has to be converted to 0x7fbfffff in the MIPS target code.

OK, the values that can cause a problem is all values that would have a zero mantissa once rounded to sinlge-precision. As the PowerPC requires that the result would have a zero mantissa (and the result class set to

qNan), I can see no way to handle this case in the generic code. And even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC code would not be able to make the distinction between infinity case and qNaN case. Then, the only solution, as you already mentioned, is to check for qNaN before calling the rounding function. As the target emulation code already has to check for sNaN to be able to raise an exception when it's needed, checking for qNaN would cost nothing more;

just have to change the check if (float64_is_signaling_nan) check with a check for NaN and handle the two cases by hand. I can see no other way to have all cases handled for all targets specific cases, do you ?

Quoted 45 lines · J. Mayer

Quoted 16 lines · Aurelien Jarno

Quoted 15 lines · J. Mayer

Quoted 14 lines · Aurelien Jarno

Quoted 13 lines · J. Mayer

Quoted 12 lines · Aurelien Jarno

Quoted 11 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

[...]

Quoted 26 lines

Quoted 11 lines

Quoted 7 lines

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

I don't say you have to return an infinity when the argument is a qNaN. I just say you have to return a qNaN in a generic way. Just return sign | 0x7f800000 | mantissa, which is the more generic form and seems to me to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

Quoted 7 lines

PowerPC) that specify that the result have to be equal to the operand (in the single precision format, of course) in such a case. This is simpler, it ensures that any target could then detect the presence of a NaN, know which one, and can then adjust the value according to its specification if needed.
I then still can'tl see any reason of having target specific code in that area.

Ok, let's give an example then. On MIPS let's say you want to convert 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to the right become 0, so you have to generate a new value. As you proposed, let's generate a "generic value" 0x7fc00000 in the softfloat routines. This value has to be converted to 0x7fbfffff in the MIPS target code.

Quoted 3 lines

OK, the values that can cause a problem is all values that would have a zero mantissa once rounded to sinlge-precision. As the PowerPC requires that the result would have a zero mantissa (and the result class set to

Are you sure of that? According to IEEE 754 a zero mantissa is not a NaN. And tests on a real machine shows different results. 0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.

Quoted 7 lines

qNan), I can see no way to handle this case in the generic code. And even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC code would not be able to make the distinction between infinity case and qNaN case. Then, the only solution, as you already mentioned, is to check for qNaN before calling the rounding function. As the target emulation code already has to check for sNaN to be able to raise an exception when it's needed, checking for qNaN would cost nothing more;

Except this is currently done *after* the call to the rounding function, using the flags returned by the softmmu routines. Doing a check before and after would slow down the emulation.

Quoted 4 lines

just have to change the check if (float64_is_signaling_nan) check with a check for NaN and handle the two cases by hand. I can see no other way to have all cases handled for all targets specific cases, do you ?

If you can confirm that the all PowerPC CPU are IEEE compliant and should behave like the 740/750, the patch I proposed is another way to be correct on all target, including PowerPC. But it seems you don't really like to add target specific code in the softmmu routines.

Quoted 45 lines · Aurelien Jarno

Quoted 44 lines · J. Mayer

Quoted 16 lines · Aurelien Jarno

Quoted 15 lines · J. Mayer

Quoted 14 lines · Aurelien Jarno

Quoted 13 lines · J. Mayer

Quoted 12 lines · Aurelien Jarno

Quoted 11 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

[...]

Quoted 26 lines

Quoted 11 lines

Quoted 7 lines

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

I don't say you have to return an infinity when the argument is a qNaN. I just say you have to return a qNaN in a generic way. Just return sign | 0x7f800000 | mantissa, which is the more generic form and seems to me to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

Quoted 7 lines

PowerPC) that specify that the result have to be equal to the operand (in the single precision format, of course) in such a case. This is simpler, it ensures that any target could then detect the presence of a NaN, know which one, and can then adjust the value according to its specification if needed.
I then still can'tl see any reason of having target specific code in that area.

Ok, let's give an example then. On MIPS let's say you want to convert 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to the right become 0, so you have to generate a new value. As you proposed, let's generate a "generic value" 0x7fc00000 in the softfloat routines. This value has to be converted to 0x7fbfffff in the MIPS target code.

Quoted 6 lines

Quoted 3 lines

OK, the values that can cause a problem is all values that would have a zero mantissa once rounded to sinlge-precision. As the PowerPC requires that the result would have a zero mantissa (and the result class set to

Are you sure of that? According to IEEE 754 a zero mantissa is not a NaN. And tests on a real machine shows different results. 0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.

First, please note that a PowerPC do not have any single precision register nor internal representation. The operation here is "round to single precision" (frsp) but the result is still a 64 bits float. Then the result is more likely to be 0x7fc0000000000000. 0x7FF0000000000001 seems to be a SNaN, according to what I see in the PowerPC specification. Then the result is OK: when no exception is raised, SNaN is converted to QNaN during rounding to single operation (please see below).
What about 0x7FF8000000000001, which is a QNaN ? According to the

PowerPC specification, this should be rounded to 0x7FF8000000000000 which is also a QNaN, then is also OK. Then rounding the mantissa and copying sign || exponent || mantissa would, in fact, always be OK in the PowerPC case.
What seem to appear to me now is that the problems are due to the fact Mips have an inverted representation of SNaN / QNaN (if I understood well) that do not allow distinction between a rounded QNaN and an infinity...

Quoted 10 lines

Quoted 7 lines

qNan), I can see no way to handle this case in the generic code. And even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC code would not be able to make the distinction between infinity case and qNaN case. Then, the only solution, as you already mentioned, is to check for qNaN before calling the rounding function. As the target emulation code already has to check for sNaN to be able to raise an exception when it's needed, checking for qNaN would cost nothing more;

Except this is currently done *after* the call to the rounding function, using the flags returned by the softmmu routines. Doing a check before and after would slow down the emulation.

On PowerPC at least, you have to check operands for sNaN _before_ doing any floating-point operation and check the result _after_ having done the floating-point computation in order to set the flags.

The sNaN operand exception must be raised before any computation is done, because it's related to the operation operands which may be lost during the operation if the target register is the same than an operand one.

The other exceptions, based on the result of the operation, must be raised after the result has been computed and stored.
Then, I can see way not to check for SNaN first. And I guess this is the case for most FPU...

Quoted 8 lines

Quoted 4 lines

just have to change the check if (float64_is_signaling_nan) check with a check for NaN and handle the two cases by hand. I can see no other way to have all cases handled for all targets specific cases, do you ?

If you can confirm that the all PowerPC CPU are IEEE compliant and should behave like the 740/750, the patch I proposed is another way to be correct on all target, including PowerPC. But it seems you don't really like to add target specific code in the softmmu routines.

The code you proposed is not correct for PowerPC. Here's what the PowerPC specification says:
Just a few precisions before:
* bit 0 is MSB in IBM representation
* notation (XXX)m:n means bits from m to n (included) of XXX
* FRB is the operand register
* FRT is the result register
* FPSCR is the FPU flags register

Floating point round to single-precision model:
(just keeping the NaN parts and adding comments, hope they are releveant !)
[...]
if ((FRB) 1:11 == 2047) then

    if ((FRB)12:63 == 0) then goto Infinity Operand /* zero mantissa */
    if ((FRB)12 == 1) then goto QNaN Operand           /* mantissa MSB

is one */
if ((FRB)12 == 0) and ((FRB)13:63 > 0) then goto SNaN Operand /*
mantissa MSB is zero and mantissa != 0 */

[...]

QNaN Operand:
FRT <- (FRB)0:34 || 0 /* Copy the highest 35 bits of the operand to
result */
(FPSCR)FPRF <- QNaN /* Set result class to QNaN */
(FPSCR)FRFI <- 0b00 /* Reset result sign / zero flags */

SNaN Operand:
1 <- (FPSCR)VXSNAN <- 1 /* Set SNaN exception flag */ if ((FPSCR)VE == 0) then /* SNaN exceptions are disabled */

    (FRT)0:11 <- (FRB)0:11 /* Copy sign / exponent from the operand to

result */
(FRT)12 <- 1 /* Ensure the the highest bit of mantissa is 1 */ (FRT)13:63 <- (FRB)13:34 || 0 /* Copy the bits 13:34 from the
operand to result */
(FPSCR)FPRF <- QNaN /* set the result class to QNaN */
(FPSCR)FRFI <- 0b00 /* Reset result sign / zero flags */

Then, as mentionned previously, there is no problem in this model, except the fact that it keeps more bits than the ones that would fit in a single-precision float in case of NaN: it keeps 35 bits of the operand, then this cannot be handled by the generic routine that would just keep 32 bits... Then I will still have to check this case at the same place I check for sNaN in order to always have the correct result. If the SNaN / QNaN representation is inverted, compared to this model, then yes, you have a problem with this method. But I still don't think adding target specific code in generic code that should not be target dependant is the solution. Having a define signaling that QNaN is 1/0 // SNaN is 0/1 would be more generic and checking that flag (and not TARGET_XXX) in that code would be more acceptable. But I keep thinking that a check on the operand is needed before computation for most CPUs, then this hack should not be needed...

From my point of view the best is to create a "round to single

Quoted 62 lines · J. Mayer

Quoted 45 lines · Aurelien Jarno

Quoted 44 lines · J. Mayer

Quoted 16 lines · Aurelien Jarno

Quoted 15 lines · J. Mayer

Quoted 14 lines · Aurelien Jarno

Quoted 13 lines · J. Mayer

Quoted 12 lines · Aurelien Jarno

Quoted 11 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

[...]

Quoted 26 lines

Quoted 11 lines

Quoted 7 lines

Anyway there is no way to do that in the target specific code *after the conversion*, as the detection of a mantissa being nul when converting from double to single precision can only be done when both values are still known. In other words when the value is not fixed during the conversion, the value 0x7f800000 can either be infinity or a conversion of NaN from double to single precision, and thus is it not possible to fix the value afterwards in the target specific code.

I don't say you have to return an infinity when the argument is a qNaN. I just say you have to return a qNaN in a generic way. Just return sign | 0x7f800000 | mantissa, which is the more generic form and seems to me to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

Quoted 7 lines

PowerPC) that specify that the result have to be equal to the operand (in the single precision format, of course) in such a case. This is simpler, it ensures that any target could then detect the presence of a NaN, know which one, and can then adjust the value according to its specification if needed.
I then still can'tl see any reason of having target specific code in that area.

Ok, let's give an example then. On MIPS let's say you want to convert 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to the right become 0, so you have to generate a new value. As you proposed, let's generate a "generic value" 0x7fc00000 in the softfloat routines. This value has to be converted to 0x7fbfffff in the MIPS target code.

Quoted 6 lines

Quoted 3 lines

OK, the values that can cause a problem is all values that would have a zero mantissa once rounded to sinlge-precision. As the PowerPC requires that the result would have a zero mantissa (and the result class set to

Are you sure of that? According to IEEE 754 a zero mantissa is not a NaN. And tests on a real machine shows different results. 0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.

First, please note that a PowerPC do not have any single precision register nor internal representation. The operation here is "round to single precision" (frsp) but the result is still a 64 bits float. Then the result is more likely to be 0x7fc0000000000000. 0x7FF0000000000001 seems to be a SNaN, according to what I see in the PowerPC specification. Then the result is OK: when no exception is raised, SNaN is converted to QNaN during rounding to single operation (please see below).
What about 0x7FF8000000000001, which is a QNaN ? According to the

Quoted 8 lines

PowerPC specification, this should be rounded to 0x7FF8000000000000 which is also a QNaN, then is also OK. Then rounding the mantissa and copying sign || exponent || mantissa would, in fact, always be OK in the PowerPC case.
What seem to appear to me now is that the problems are due to the fact Mips have an inverted representation of SNaN / QNaN (if I understood well) that do not allow distinction between a rounded QNaN and an infinity...

Nope it is not due to the fact that MIPS uses an "inverted" representation. It is the same problem on x86 or other target, except that they can allow the distinction between a rounded SNaN and an infinity. The problem is present on all targets that can represent a single precision FP

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

qNan), I can see no way to handle this case in the generic code. And even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC code would not be able to make the distinction between infinity case and qNaN case. Then, the only solution, as you already mentioned, is to check for qNaN before calling the rounding function. As the target emulation code already has to check for sNaN to be able to raise an exception when it's needed, checking for qNaN would cost nothing more;

Except this is currently done *after* the call to the rounding function, using the flags returned by the softmmu routines. Doing a check before and after would slow down the emulation.

Quoted 8 lines

On PowerPC at least, you have to check operands for sNaN _before_ doing any floating-point operation and check the result _after_ having done the floating-point computation in order to set the flags.

The sNaN operand exception must be raised before any computation is done, because it's related to the operation operands which may be lost during the operation if the target register is the same than an operand one.

I don't really understand why this is necessary to do it before *and* after. The softmmu routines already does that job, and compute the correct flags according to the operand. The only remaining operation for the target specific code is to copy the softmmu flags to the target FP status register and trigger exception if necessary.

Anyway that's not the point here, what to remember is that most target only check operand after the softmmu routine

Quoted 15 lines

The other exceptions, based on the result of the operation, must be raised after the result has been computed and stored.
Then, I can see way not to check for SNaN first. And I guess this is the case for most FPU...

Quoted 8 lines

Quoted 4 lines

just have to change the check if (float64_is_signaling_nan) check with a check for NaN and handle the two cases by hand. I can see no other way to have all cases handled for all targets specific cases, do you ?

If you can confirm that the all PowerPC CPU are IEEE compliant and should behave like the 740/750, the patch I proposed is another way to be correct on all target, including PowerPC. But it seems you don't really like to add target specific code in the softmmu routines.

The code you proposed is not correct for PowerPC.

It looks like the problem is different on PowerPC, because it does not know about single precision FP. Then I wonder why to use "double precision to single precision conversion" softmmu routine on PowerPC. This routine is working correctly (with the patch I submitted) for all target, but PowerPC.

Quoted 1 line

From my point of view the best is to create a "round to single

precision" softmmu subroutine that takes a double precision for both input and output for PowerPC.

Quoted 9 lines · Daniel Jacobowitz

Quoted 8 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

Quoted 9 lines

Quoted 2 lines

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v retrieving revision 1.3
diff -u -d -p -r1.3 softfloat-specialize.h

softfloat-specialize.h +1 -3

@@ -120,9 +120,17 @@ static commonNaNT float32ToCommonNaN( fl
 
 static float32 commonNaNToFloat32( commonNaNT a )
 {
-
-    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
-
+    bits32 mantissa = a.high>>41;

Quoted 18 lines · Aurelien Jarno

Quoted 17 lines · Daniel Jacobowitz

Quoted 10 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that. It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it. Andreas reminded me that we need to make sure at least one mantissa bit is set. If we're confident that the common NaN format will already have some bit other than the qnan/snan bit set, this is fine; otherwise, we might want to forcibly set some other mantissa bit.

Quoted 2 lines

Please find an updated patch below. I have tried to match real x86, MIPS, HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

The default NaN pattern for MIPS and HPPA were broken. (Fabrice spotted this.) I currently have the appended patch, it assumes HPPA is similiar to MIPS, which is probably incorrect.

Quoted 20 lines

Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v retrieving revision 1.3
diff -u -d -p -r1.3 softfloat-specialize.h

softfloat-specialize.h +6 -3

@@ -120,9 +120,17 @@ static commonNaNT float32ToCommonNaN( fl
 
 static float32 commonNaNToFloat32( commonNaNT a )
 {
-
-    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
-
+    bits32 mantissa = a.high>>41;
+    if (mantissa)
+        return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( mantissa );
+    else
+#if defined(TARGET_MIPS)
+        return ( ( (bits32) a.sign )<<31 ) | 0x7FBFFFFF | ( mantissa );

softfloat-specialize.h static commonNaNT float32ToCommonNaN( fl · line 133

Quoted 5 lines

softfloat-specialize.h +5 -0

+#elif defined(TARGET_HPPA)
+        return ( ( (bits32) a.sign )<<31 ) | 0x7FA00000 | ( mantissa );
+#else
+        return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000;
+#endif

This looks odd. Do MIPS and HPPA really need a specialcase here but none for doubles? Could you re-check with my patch applied?

I have looked quickly at your patch, it looks like the right approach to define target specific things at only one place in the file. I will get a closer tomorrow.

Quoted 4 lines

I have looked quickly at your patch, it looks like the right approach to define target specific things at only one place in the file. I will get a closer tomorrow.

Sorry I have been busy with real life stuff, and haven't looked at your patch. I have seen you have committed it.

I have rebased the patch I already posted against the current CVS, it is less intrusive now. It defines float32_default_nan and float64_default_nan for the various targets, as it varies even within targets which have the same convention for the SNAN bit.

It also keeps the mantissa when converting FP numbers, so that qNaN are not changed into sNaN on MIPS and HPPA. If the mantissa becames zero a new NaN number is generated.

It finally fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v retrieving revision 1.5
diff -u -d -p -r1.5 softfloat-specialize.h

softfloat-specialize.h +35 -11

@@ -65,7 +65,13 @@ typedef struct {
 /*----------------------------------------------------------------------------
 | The pattern for a default generated single-precision NaN.
 *----------------------------------------------------------------------------*/
-#if SNAN_BIT_IS_ONE
+#if defined(TARGET_SPARC)
+#define float32_default_nan make_float32(0x7FFFFFFF)
+#elif defined(TARGET_POWERPC)
+#define float32_default_nan make_float32(0x7FC00000)
+#elif defined(TARGET_HPPA)
+#define float32_default_nan make_float32(0x7FA00000)
+#elif SNAN_BIT_IS_ONE
 #define float32_default_nan make_float32(0x7FBFFFFF)
 #else
 #define float32_default_nan make_float32(0xFFC00000)

@@ -125,8 +131,12 @@ static commonNaNT float32ToCommonNaN( fl
 
 static float32 commonNaNToFloat32( commonNaNT a )
 {
-    return make_float32(
-        ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 ) );
+    bits32 mantissa = a.high>>41;
+    if ( mantissa )
+        return make_float32(
+            ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 ) );
+    else
+        return float32_default_nan;
 }
 
 /*----------------------------------------------------------------------------

@@ -180,7 +190,13 @@ static float32 propagateFloat32NaN( floa
 /*----------------------------------------------------------------------------
 | The pattern for a default generated double-precision NaN.
 *----------------------------------------------------------------------------*/
-#if SNAN_BIT_IS_ONE
+#if defined(TARGET_SPARC)
+#define float64_default_nan make_float64(LIT64( 0x7FFFFFFFFFFFFFFF ))
+#elif defined(TARGET_POWERPC)
+#define float64_default_nan make_float64(LIT64( 0x7FF8000000000000 ))
+#elif defined(TARGET_HPPA)
+#define float64_default_nan make_float64(LIT64( 0x7FF4000000000000 ))
+#elif SNAN_BIT_IS_ONE
 #define float64_default_nan make_float64(LIT64( 0x7FF7FFFFFFFFFFFF ))
 #else
 #define float64_default_nan make_float64(LIT64( 0xFFF8000000000000 ))

@@ -244,10 +260,15 @@ static commonNaNT float64ToCommonNaN( fl
 
 static float64 commonNaNToFloat64( commonNaNT a )
 {
-    return make_float64(
-          ( ( (bits64) a.sign )<<63 )
-        | LIT64( 0x7FF8000000000000 )
-        | ( a.high>>12 ));
+    bits64 mantissa = a.high>>12;
+
+    if ( mantissa )
+        return make_float64(
+              ( ( (bits64) a.sign )<<63 )
+            | LIT64( 0x7FF0000000000000 )
+            | ( a.high>>12 ));
+    else
+        return float64_default_nan;
 }
 
 /*----------------------------------------------------------------------------

@@ -366,7 +387,7 @@ static commonNaNT floatx80ToCommonNaN( f
     if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
     z.sign = a.high>>15;
     z.low = 0;
-    z.high = a.low<<1;
+    z.high = a.low;
     return z;
 }
 

@@ -379,7 +400,10 @@ static floatx80 commonNaNToFloatx80( com
 {
     floatx80 z;
 
-    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
+    if (a.high)
+        z.low = a.high;
+    else
+        z.low = floatx80_default_nan_low;
     z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
     return z;
 }

@@ -500,7 +524,7 @@ static float128 commonNaNToFloat128( com
     float128 z;
 
     shift128Right( a.high, a.low, 16, &z.high, &z.low );
-    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
+    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
     return z;
 }
 

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that.

It also fixes an off-by-one in the extended double precision format (aka floatx80), the mantissa is 64-bit long and not 63-bit long.

With this patch applied all the glibc 2.7 floating point tests are successfull on MIPS and MIPSEL.

Bye,
Aurelien

Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v retrieving revision 1.3
diff -u -d -p -r1.3 softfloat-specialize.h

softfloat-specialize.h +5 -6

@@ -121,8 +121,7 @@ static commonNaNT float32ToCommonNaN( fl
 static float32 commonNaNToFloat32( commonNaNT a )
 {
 
-    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
-
+    return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 );
 }
 
 /*----------------------------------------------------------------------------

@@ -233,7 +232,7 @@ static float64 commonNaNToFloat64( commo
 
     return
           ( ( (bits64) a.sign )<<63 )
-        | LIT64( 0x7FF8000000000000 )
+        | LIT64( 0x7FF0000000000000 )
         | ( a.high>>12 );
 
 }

@@ -329,7 +328,7 @@ static commonNaNT floatx80ToCommonNaN( f
     if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
     z.sign = a.high>>15;
     z.low = 0;
-    z.high = a.low<<1;
+    z.high = a.low;
     return z;
 
 }

@@ -343,7 +342,7 @@ static floatx80 commonNaNToFloatx80( com
 {
     floatx80 z;
 
-    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
+    z.low = a.high;
     z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
     return z;
 

@@ -449,7 +448,7 @@ static float128 commonNaNToFloat128( com
     float128 z;
 
     shift128Right( a.high, a.low, 16, &z.high, &z.low );
-    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
+    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
     return z;
 
 }

Signature · 5 lines

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   [email protected]         | [email protected]
   `-    people.debian.org/~aurel32 | www.aurel32.net

Quoted 5 lines · Aurelien Jarno

Hi all,

The current softfloat implementation changes qNaN into sNaN when converting between formats, for no reason. The attached patch fixes that.

Did you take into account that MIPS and PA-RISC have the signalling bit inverted to the rest of the world?