From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Iqz4R-0003oU-6M for qemu-devel@nongnu.org; Sat, 10 Nov 2007 17:44:55 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Iqz4Q-0003nz-8n for qemu-devel@nongnu.org; Sat, 10 Nov 2007 17:44:54 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Iqz4Q-0003nk-5P for qemu-devel@nongnu.org; Sat, 10 Nov 2007 17:44:54 -0500 Received: from bangui.magic.fr ([195.154.194.245]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Iqz4P-00074e-3f for qemu-devel@nongnu.org; Sat, 10 Nov 2007 17:44:53 -0500 Subject: Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat From: "J. Mayer" In-Reply-To: <4735F3C7.1050302@aurel32.net> References: <20071103173548.GA16847@hall.aurel32.net> <20071103180604.GA14403@caradoc.them.org> <20071103212816.GA31686@hall.aurel32.net> <1194379274.31210.100.camel@rapid> <20071107230525.GG19509@hall.aurel32.net> <1194647498.21588.79.camel@rapid> <47357B6A.1020407@aurel32.net> <1194701464.21588.94.camel@rapid> <4735D921.4000407@aurel32.net> <1194714847.21588.155.camel@rapid> <4735F3C7.1050302@aurel32.net> Content-Type: text/plain; charset=ISO-8859-15 Date: Sat, 10 Nov 2007 23:44:41 +0100 Message-Id: <1194734681.21588.177.camel@rapid> Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org On Sat, 2007-11-10 at 19:09 +0100, Aurelien Jarno wrote: > J. Mayer a =E9crit : > > On Sat, 2007-11-10 at 17:15 +0100, Aurelien Jarno wrote: > >> J. Mayer a =E9crit : > >>> On Sat, 2007-11-10 at 10:35 +0100, Aurelien Jarno wrote: > >>>> J. Mayer a =E9crit : > >>>>> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote: > >>>>>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote: > >>>>>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote:=20 > >>>>>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wr= ote: > >>>>>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrot= e: > >>>>>>>>>> Hi all, > >>>>>>>>>> > >>>>>>>>>> The current softfloat implementation changes qNaN into sNaN = when=20 > >>>>>>>>>> converting between formats, for no reason. The attached patc= h fixes > >>>>>>>>>> that. It also fixes an off-by-one in the extended double pre= cision > >>>>>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 6= 3-bit > >>>>>>>>>> long. > >>>>>>>>>> > >>>>>>>>>> With this patch applied all the glibc 2.7 floating point tes= ts > >>>>>>>>>> are successfull on MIPS and MIPSEL. > >>> [...] > >>>>>> Anyway there is no way to do that in the target specific code *a= fter=20 > >>>>>> the conversion*, as the detection of a mantissa being nul when=20 > >>>>>> converting from double to single precision can only be done when= both > >>>>>> values are still known. In other words when the value is not fix= ed=20 > >>>>>> during the conversion, the value 0x7f800000 can either be infini= ty or a > >>>>>> conversion of NaN from double to single precision, and thus is i= t 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 retu= rn 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. > >>>> > >>>>> PowerPC) that specify that the result have to be equal to the ope= rand > >>>>> (in the single precision format, of course) in such a case. This = is > >>>>> simpler, it ensures that any target could then detect the presenc= e of a > >>>>> NaN, know which one, and can then adjust the value according to i= ts > >>>>> 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 conv= ert > >>>> 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifte= d 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 softf= loat > >>>> 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 ha= ve a > >>> zero mantissa once rounded to sinlge-precision. As the PowerPC requ= ires > >>> that the result would have a zero mantissa (and the result class se= t 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. > >=20 > > 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. The= n > > 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 fac= t > > Mips have an inverted representation of SNaN / QNaN (if I understood > > well) that do not allow distinction between a rounded QNaN and an > > infinity... >=20 > 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=20 For those targets that can make the distinction between rounded sNaN and infinite case, I don't see why there should be a problem. But, I realize now, as you, that PowerPC is really not a good example for comparison as it does not know about 32 bits floats. And I also realize it cannot use the softmmu routines to round 64 bits floats to single precision. > >>> qNan), I can see no way to handle this case in the generic code. An= d > >>> even adding a "#ifdef TARGET_PPC" won't solve the problem as the Po= werPC > >>> code would not be able to make the distinction between infinity cas= e 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 mo= re; > >> Except this is currently done *after* the call to the rounding funct= ion, > >> using the flags returned by the softmmu routines. Doing a check befo= re > >> and after would slow down the emulation. > >=20 > > On PowerPC at least, you have to check operands for sNaN _before_ doi= ng > > 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 los= t > > during the operation if the target register is the same than an opera= nd > > one. >=20 > 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 fo= r > the target specific code is to copy the softmmu flags to the target FP > status register and trigger exception if necessary. When using the native softfloat implementation, you don't have any flags set then cannot raise any sNaN exception. This check allow use of the native softfloat. I also checked the softfloat implementation (not the native one) for fmul and fdiv and it appears that it does not give me the needed informations to raise the correct exceptions for the PowerPC targets. For example, it does not seem to me that the 0 x infinity case is properly trapped And it does not seem to me to make a distinction between NaN operands and invalid operations or between different "classes" of invalid operations then do not allow a proper update of the PowerPC FPU flags. Then, for PowerPC, checking the precise case "by hand" and using the native softfloat implementation seems more accurate if I want to have precise and correct results (this is sure not the case, due to all the bugs I must have accumulated in that code !). > Anyway that's not the point here, what to remember is that most target > only check operand after the softmmu routine >=20 > > 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... > >=20 > >>> just have to change the check if (float64_is_signaling_nan) check w= ith 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. > >=20 > > 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. >=20 > >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. Yes, you're right, PowerPC have to use its own conversion routine. And it seems it also needs specific routines to load and store single precision floats to / from double precision registers. --=20 J. Mayer Never organized