[PATCH] [APFloat] x-NaN needs to flip sign of NaN

Michael Gottesman mgottesman at apple.com
Sun Jun 8 09:41:10 PDT 2014


Thanks for updating the tests Steve. Also your patch LGTM except you need to change the C style comment to a C++ comment. In general in LLVM we only use C style comments in C source files.

To give some background to the list. There is a bug somewhere in APFloat that breaks commutativity with NaN when adding (IIRC I documented it in the unittests where we are checking that behavior).

In order to work around this inconsistency (and the lack of time I had at the moment to look into it), I took advantage of the clause in IEEE-754R which states that the semantic meaning of the sign bit of NaN is not specified. So I changed APFloat’s arithmetic operations to always emit a positive NaN since I could define the sign bit of a NaN to be semantically meaningless. But this creates an interesting issue (to quote Steve):

> Whereas C, C++, and IEEE-754 do not care about the sign of NaN *in most contexts* (in fact, IEEE-754 says it has no semantic meaning in general), there are four "signbit" operations defined in IEEE-754 which are defined to "only effect the signbit" and "treating floating-point numbers and NaNs alike".  I.e. when used in one of these four operations, the sign of NaN *does* have semantic meaning:
> 
> 	z = copy(x)
> 	z = negate(x)
> 	z = abs(x)
> 	z = copySign(x,y)
> 
> Those are the IEEE-754 names for the operations; the C and C++ mappings for them are assignment without implicit conversion, unary minus, fabs( ) and copysign( ), respectively.  The trouble is that we don't have a "negate" operation; instead –x becomes 0 – x, which means that we need to make subtraction of NaN flip the sign bit in order to get the right behavior for negation.

Since subtraction is never commutative this will not expose the commutativity bug so the change gives us more correctness without exposing further bugs in APFloat.

Thanks Steve!
Michael

On Jun 7, 2014, at 11:44 PM, Steve (Numerics) Canon <scanon at apple.com> wrote:

> On Jun 7, 2014, at 11:25 PM, Alp Toker <alp at nuanti.com> wrote:
> 
>> On 08/06/2014 09:18, Steve (Numerics) Canon wrote:
>>> This is simple enough for review-after-commit, but I don't often work on llvm, so I thought it best to send for review.
>>> 
>>> This patch fixes http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-June/037388.html; it turned out to be caused by APFloat not negating the sign of the NaN when producing the result of (0 – NaN).  I made it flip the sign for x – NaN where x is any number.  This isn't strictly required, but is allowed by IEEE-754, and was the minimal source change to get the needed behavior for the 0 – NaN case.
>>> 
>>> I have no idea how to write a test case for this bug; APFloat seems to be, ahem, "sparsely tested" at present, so there isn't much stylistic precedent.
>> 
>> Hi Steve,
>> 
>> This is one of those (rare) cases where a unit test is appropriate. Could you take a look at unittests/ADT/APFloatTest.cpp?
>> 
>> Alp.
> 
> Ah!  Thanks for the tip.
> 
> Now with tests:
> 
> <APFloat-negate.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140608/e432027e/attachment.html>


More information about the llvm-commits mailing list